From 74fa746ce835d1a2f422a880a9ab3334b853ebda Mon Sep 17 00:00:00 2001
From: Pedro Pombeiro <noreply@pedro.pombei.ro>
Date: Fri, 23 Jul 2021 16:14:22 +0200
Subject: [PATCH 01/10] Change RunnersFinder to take group in params hash

This will standardize the way that GraphQL resolvers pass arguments
to the finder.
---
 app/controllers/groups/runners_controller.rb        |  2 +-
 app/controllers/groups/settings/ci_cd_controller.rb |  2 +-
 app/finders/ci/runners_finder.rb                    | 13 +++++++------
 spec/finders/ci/runners_finder_spec.rb              |  4 ++--
 4 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/app/controllers/groups/runners_controller.rb b/app/controllers/groups/runners_controller.rb
index dbbfdd76fe8bfe..ff3a09a2d2d89a 100644
--- a/app/controllers/groups/runners_controller.rb
+++ b/app/controllers/groups/runners_controller.rb
@@ -59,7 +59,7 @@ def pause
   private
 
   def runner
-    @runner ||= Ci::RunnersFinder.new(current_user: current_user, group: @group, params: {}).execute
+    @runner ||= Ci::RunnersFinder.new(current_user: current_user, params: { group: @group }).execute
                                                                                             .except(:limit, :offset)
                                                                                             .find(params[:id])
   end
diff --git a/app/controllers/groups/settings/ci_cd_controller.rb b/app/controllers/groups/settings/ci_cd_controller.rb
index 0f40c9bfd2cb8f..a290ef9b5e7b84 100644
--- a/app/controllers/groups/settings/ci_cd_controller.rb
+++ b/app/controllers/groups/settings/ci_cd_controller.rb
@@ -17,7 +17,7 @@ class CiCdController < Groups::ApplicationController
       NUMBER_OF_RUNNERS_PER_PAGE = 4
 
       def show
-        runners_finder = Ci::RunnersFinder.new(current_user: current_user, group: @group, params: params)
+        runners_finder = Ci::RunnersFinder.new(current_user: current_user, params: params.merge({ group: @group }))
         # We need all runners for count
         @all_group_runners = runners_finder.execute.except(:limit, :offset)
         @group_runners = runners_finder.execute.page(params[:page]).per(NUMBER_OF_RUNNERS_PER_PAGE)
diff --git a/app/finders/ci/runners_finder.rb b/app/finders/ci/runners_finder.rb
index d34b32024335a7..1ea1c6adb9b039 100644
--- a/app/finders/ci/runners_finder.rb
+++ b/app/finders/ci/runners_finder.rb
@@ -7,9 +7,8 @@ class RunnersFinder < UnionFinder
     ALLOWED_SORTS = %w[contacted_asc contacted_desc created_at_asc created_at_desc created_date].freeze
     DEFAULT_SORT = 'created_at_desc'
 
-    def initialize(current_user:, group: nil, params:)
+    def initialize(current_user:, params:)
       @params = params
-      @group = group
       @current_user = current_user
     end
 
@@ -34,7 +33,7 @@ def sort_key
     private
 
     def search!
-      @group ? group_runners : all_runners
+      @params.has_key?(:group) ? group_runners : all_runners
 
       @runners = @runners.search(@params[:search]) if @params[:search].present?
     end
@@ -46,12 +45,14 @@ def all_runners
     end
 
     def group_runners
-      raise Gitlab::Access::AccessDeniedError unless can?(@current_user, :admin_group, @group)
+      group = @params[:group]
+
+      raise Gitlab::Access::AccessDeniedError unless can?(@current_user, :admin_group, group)
 
       # Getting all runners from the group itself and all its descendants
-      descendant_projects = Project.for_group_and_its_subgroups(@group)
+      descendant_projects = Project.for_group_and_its_subgroups(group)
 
-      @runners = Ci::Runner.belonging_to_group_or_project(@group.self_and_descendants, descendant_projects)
+      @runners = Ci::Runner.belonging_to_group_or_project(group.self_and_descendants, descendant_projects)
     end
 
     def filter_by_status!
diff --git a/spec/finders/ci/runners_finder_spec.rb b/spec/finders/ci/runners_finder_spec.rb
index 599b4ffb8046c2..a20cd5c681d96f 100644
--- a/spec/finders/ci/runners_finder_spec.rb
+++ b/spec/finders/ci/runners_finder_spec.rb
@@ -182,7 +182,7 @@
     end
 
     describe '#execute' do
-      subject { described_class.new(current_user: user, group: group, params: params).execute }
+      subject { described_class.new(current_user: user, params: params.merge(group: group)).execute }
 
       context 'with user as group owner' do
         before do
@@ -278,7 +278,7 @@
     end
 
     describe '#sort_key' do
-      subject { described_class.new(current_user: user, group: group, params: params).sort_key }
+      subject { described_class.new(current_user: user, params: params.merge(group: group)).sort_key }
 
       context 'without params' do
         it 'returns created_at_desc' do
-- 
GitLab


From bfad04ee1513d8311498bf77a7c57baff325dc2e Mon Sep 17 00:00:00 2001
From: Pedro Pombeiro <noreply@pedro.pombei.ro>
Date: Fri, 23 Jul 2021 16:16:25 +0200
Subject: [PATCH 02/10] Extract RunnersResolver context and examples

This will allow the upcoming derived GroupRunnersResolver to reuse the
shared context and examples.
---
 .../resolvers/ci/runners_resolver_spec.rb     | 183 +-----------------
 .../runners_resolver_shared_context.rb        |  22 +++
 .../runners_resolver_shared_examples.rb       | 175 +++++++++++++++++
 3 files changed, 199 insertions(+), 181 deletions(-)
 create mode 100644 spec/support/shared_contexts/graphql/resolvers/runners_resolver_shared_context.rb
 create mode 100644 spec/support/shared_examples/graphql/resolvers/runners_resolver_shared_examples.rb

diff --git a/spec/graphql/resolvers/ci/runners_resolver_spec.rb b/spec/graphql/resolvers/ci/runners_resolver_spec.rb
index 5ac15d5729ffd0..588ca82aa2fe50 100644
--- a/spec/graphql/resolvers/ci/runners_resolver_spec.rb
+++ b/spec/graphql/resolvers/ci/runners_resolver_spec.rb
@@ -5,186 +5,7 @@
 RSpec.describe Resolvers::Ci::RunnersResolver do
   include GraphqlHelpers
 
-  let_it_be(:user) { create_default(:user, :admin) }
-  let_it_be(:group) { create(:group, :public) }
-  let_it_be(:project) { create(:project, :repository, :public) }
+  include_context 'runners resolver setup'
 
-  let_it_be(:inactive_project_runner) do
-    create(:ci_runner, :project, projects: [project], description: 'inactive project runner', token: 'abcdef', active: false, contacted_at: 1.minute.ago, tag_list: %w(project_runner))
-  end
-
-  let_it_be(:offline_project_runner) do
-    create(:ci_runner, :project, projects: [project], description: 'offline project runner', token: 'defghi', contacted_at: 1.day.ago, tag_list: %w(project_runner active_runner))
-  end
-
-  let_it_be(:group_runner) { create(:ci_runner, :group, groups: [group], token: 'mnopqr', description: 'group runner', contacted_at: 1.second.ago) }
-  let_it_be(:instance_runner) { create(:ci_runner, :instance, description: 'shared runner', token: 'stuvxz', contacted_at: 2.minutes.ago, tag_list: %w(instance_runner active_runner)) }
-
-  describe '#resolve' do
-    subject { resolve(described_class, ctx: { current_user: user }, args: args).items.to_a }
-
-    let(:args) do
-      {}
-    end
-
-    context 'when the user cannot see runners' do
-      let(:user) { create(:user) }
-
-      it 'returns no runners' do
-        is_expected.to be_empty
-      end
-    end
-
-    context 'without sort' do
-      it 'returns all the runners' do
-        is_expected.to contain_exactly(inactive_project_runner, offline_project_runner, group_runner, instance_runner)
-      end
-    end
-
-    context 'with a sort argument' do
-      context "set to :contacted_asc" do
-        let(:args) do
-          { sort: :contacted_asc }
-        end
-
-        it { is_expected.to eq([offline_project_runner, instance_runner, inactive_project_runner, group_runner]) }
-      end
-
-      context "set to :contacted_desc" do
-        let(:args) do
-          { sort: :contacted_desc }
-        end
-
-        it { is_expected.to eq([offline_project_runner, instance_runner, inactive_project_runner, group_runner].reverse) }
-      end
-
-      context "set to :created_at_desc" do
-        let(:args) do
-          { sort: :created_at_desc }
-        end
-
-        it { is_expected.to eq([instance_runner, group_runner, offline_project_runner, inactive_project_runner]) }
-      end
-
-      context "set to :created_at_asc" do
-        let(:args) do
-          { sort: :created_at_asc }
-        end
-
-        it { is_expected.to eq([instance_runner, group_runner, offline_project_runner, inactive_project_runner].reverse) }
-      end
-    end
-
-    context 'when type is filtered' do
-      let(:args) do
-        { type: runner_type.to_s }
-      end
-
-      context 'to instance runners' do
-        let(:runner_type) { :instance_type }
-
-        it 'returns the instance runner' do
-          is_expected.to contain_exactly(instance_runner)
-        end
-      end
-
-      context 'to group runners' do
-        let(:runner_type) { :group_type }
-
-        it 'returns the group runner' do
-          is_expected.to contain_exactly(group_runner)
-        end
-      end
-
-      context 'to project runners' do
-        let(:runner_type) { :project_type }
-
-        it 'returns the project runner' do
-          is_expected.to contain_exactly(inactive_project_runner, offline_project_runner)
-        end
-      end
-    end
-
-    context 'when status is filtered' do
-      let(:args) do
-        { status: runner_status.to_s }
-      end
-
-      context 'to active runners' do
-        let(:runner_status) { :active }
-
-        it 'returns the instance and group runners' do
-          is_expected.to contain_exactly(offline_project_runner, group_runner, instance_runner)
-        end
-      end
-
-      context 'to offline runners' do
-        let(:runner_status) { :offline }
-
-        it 'returns the offline project runner' do
-          is_expected.to contain_exactly(offline_project_runner)
-        end
-      end
-    end
-
-    context 'when tag list is filtered' do
-      let(:args) do
-        { tag_list: tag_list }
-      end
-
-      context 'with "project_runner" tag' do
-        let(:tag_list) { ['project_runner'] }
-
-        it 'returns the project_runner runners' do
-          is_expected.to contain_exactly(offline_project_runner, inactive_project_runner)
-        end
-      end
-
-      context 'with "project_runner" and "active_runner" tags as comma-separated string' do
-        let(:tag_list) { ['project_runner,active_runner'] }
-
-        it 'returns the offline_project_runner runner' do
-          is_expected.to contain_exactly(offline_project_runner)
-        end
-      end
-
-      context 'with "active_runner" and "instance_runner" tags as array' do
-        let(:tag_list) { %w[instance_runner active_runner] }
-
-        it 'returns the offline_project_runner runner' do
-          is_expected.to contain_exactly(instance_runner)
-        end
-      end
-    end
-
-    context 'when text is filtered' do
-      let(:args) do
-        { search: search_term }
-      end
-
-      context 'to "project"' do
-        let(:search_term) { 'project' }
-
-        it 'returns both project runners' do
-          is_expected.to contain_exactly(inactive_project_runner, offline_project_runner)
-        end
-      end
-
-      context 'to "group"' do
-        let(:search_term) { 'group' }
-
-        it 'returns group runner' do
-          is_expected.to contain_exactly(group_runner)
-        end
-      end
-
-      context 'to "defghi"' do
-        let(:search_term) { 'defghi' }
-
-        it 'returns runners containing term in token' do
-          is_expected.to contain_exactly(offline_project_runner)
-        end
-      end
-    end
-  end
+  it_behaves_like Resolvers::Ci::RunnersResolver
 end
diff --git a/spec/support/shared_contexts/graphql/resolvers/runners_resolver_shared_context.rb b/spec/support/shared_contexts/graphql/resolvers/runners_resolver_shared_context.rb
new file mode 100644
index 00000000000000..aa857cfdb703d0
--- /dev/null
+++ b/spec/support/shared_contexts/graphql/resolvers/runners_resolver_shared_context.rb
@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.shared_context 'runners resolver setup' do
+  let_it_be(:user) { create_default(:user, :admin) }
+  let_it_be(:group) { create(:group, :public) }
+  let_it_be(:subgroup) { create(:group, :public, parent: group) }
+  let_it_be(:project) { create(:project, :public, group: group) }
+
+  let_it_be(:inactive_project_runner) do
+    create(:ci_runner, :project, projects: [project], description: 'inactive project runner', token: 'abcdef', active: false, contacted_at: 1.minute.ago, tag_list: %w(project_runner))
+  end
+
+  let_it_be(:offline_project_runner) do
+    create(:ci_runner, :project, projects: [project], description: 'offline project runner', token: 'defghi', contacted_at: 1.day.ago, tag_list: %w(project_runner active_runner))
+  end
+
+  let_it_be(:group_runner) { create(:ci_runner, :group, groups: [group], token: 'mnopqr', description: 'group runner', contacted_at: 2.seconds.ago) }
+  let_it_be(:subgroup_runner) { create(:ci_runner, :group, groups: [subgroup], token: 'mnopqr', description: 'subgroup runner', contacted_at: 1.second.ago) }
+  let_it_be(:instance_runner) { create(:ci_runner, :instance, description: 'shared runner', token: 'stuvxz', contacted_at: 2.minutes.ago, tag_list: %w(instance_runner active_runner)) }
+end
diff --git a/spec/support/shared_examples/graphql/resolvers/runners_resolver_shared_examples.rb b/spec/support/shared_examples/graphql/resolvers/runners_resolver_shared_examples.rb
new file mode 100644
index 00000000000000..c89d1713e03b33
--- /dev/null
+++ b/spec/support/shared_examples/graphql/resolvers/runners_resolver_shared_examples.rb
@@ -0,0 +1,175 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.shared_examples_for Resolvers::Ci::RunnersResolver do
+  describe '#resolve' do
+    subject { resolve(described_class, ctx: { current_user: user }, args: args).items.to_a }
+
+    context 'with empty args' do
+      let(:args) do
+        {}
+      end
+
+      context 'when the user cannot see runners' do
+        let(:user) { create(:user) }
+
+        it 'returns no runners' do
+          is_expected.to be_empty
+        end
+      end
+
+      context 'without sort' do
+        it 'returns all the runners' do
+          is_expected.to contain_exactly(inactive_project_runner, offline_project_runner, group_runner, subgroup_runner, instance_runner)
+        end
+      end
+    end
+
+    context 'with a sort argument' do
+      context "set to :contacted_asc" do
+        let(:args) do
+          { sort: :contacted_asc }
+        end
+
+        it { is_expected.to eq([offline_project_runner, instance_runner, inactive_project_runner, group_runner, subgroup_runner]) }
+      end
+
+      context "set to :contacted_desc" do
+        let(:args) do
+          { sort: :contacted_desc }
+        end
+
+        it { is_expected.to eq([offline_project_runner, instance_runner, inactive_project_runner, group_runner, subgroup_runner].reverse) }
+      end
+
+      context "set to :created_at_desc" do
+        let(:args) do
+          { sort: :created_at_desc }
+        end
+
+        it { is_expected.to eq([instance_runner, subgroup_runner, group_runner, offline_project_runner, inactive_project_runner]) }
+      end
+
+      context "set to :created_at_asc" do
+        let(:args) do
+          { sort: :created_at_asc }
+        end
+
+        it { is_expected.to eq([instance_runner, subgroup_runner, group_runner, offline_project_runner, inactive_project_runner].reverse) }
+      end
+    end
+
+    context 'when type is filtered' do
+      let(:args) do
+        { type: runner_type.to_s }
+      end
+
+      context 'to instance runners' do
+        let(:runner_type) { :instance_type }
+
+        it 'returns the instance runner' do
+          is_expected.to contain_exactly(instance_runner)
+        end
+      end
+
+      context 'to group runners' do
+        let(:runner_type) { :group_type }
+
+        it 'returns the group runner' do
+          is_expected.to contain_exactly(group_runner, subgroup_runner)
+        end
+      end
+
+      context 'to project runners' do
+        let(:runner_type) { :project_type }
+
+        it 'returns the project runner' do
+          is_expected.to contain_exactly(inactive_project_runner, offline_project_runner)
+        end
+      end
+    end
+
+    context 'when status is filtered' do
+      let(:args) do
+        { status: runner_status.to_s }
+      end
+
+      context 'to active runners' do
+        let(:runner_status) { :active }
+
+        it 'returns the instance and group runners' do
+          is_expected.to contain_exactly(offline_project_runner, group_runner, subgroup_runner, instance_runner)
+        end
+      end
+
+      context 'to offline runners' do
+        let(:runner_status) { :offline }
+
+        it 'returns the offline project runner' do
+          is_expected.to contain_exactly(offline_project_runner)
+        end
+      end
+    end
+
+    context 'when tag list is filtered' do
+      let(:args) do
+        { tag_list: tag_list }
+      end
+
+      context 'with "project_runner" tag' do
+        let(:tag_list) { ['project_runner'] }
+
+        it 'returns the project_runner runners' do
+          is_expected.to contain_exactly(offline_project_runner, inactive_project_runner)
+        end
+      end
+
+      context 'with "project_runner" and "active_runner" tags as comma-separated string' do
+        let(:tag_list) { ['project_runner,active_runner'] }
+
+        it 'returns the offline_project_runner runner' do
+          is_expected.to contain_exactly(offline_project_runner)
+        end
+      end
+
+      context 'with "active_runner" and "instance_runner" tags as array' do
+        let(:tag_list) { %w[instance_runner active_runner] }
+
+        it 'returns the offline_project_runner runner' do
+          is_expected.to contain_exactly(instance_runner)
+        end
+      end
+    end
+
+    context 'when text is filtered' do
+      let(:args) do
+        { search: search_term }
+      end
+
+      context 'to "project"' do
+        let(:search_term) { 'project' }
+
+        it 'returns both project runners' do
+          is_expected.to contain_exactly(inactive_project_runner, offline_project_runner)
+        end
+      end
+
+      context 'to "group"' do
+        let(:search_term) { 'group' }
+
+        it 'returns group runners' do
+          is_expected.to contain_exactly(group_runner, subgroup_runner)
+        end
+      end
+
+      context 'to "defghi"' do
+        let(:search_term) { 'defghi' }
+
+        it 'returns runners containing term in token' do
+          is_expected.to contain_exactly(offline_project_runner)
+        end
+      end
+    end
+  end
+end
-- 
GitLab


From a646f27227591cae661720dc9dbbad84512a6085 Mon Sep 17 00:00:00 2001
From: Pedro Pombeiro <noreply@pedro.pombei.ro>
Date: Fri, 23 Jul 2021 16:17:34 +0200
Subject: [PATCH 03/10] GraphQL: Add "runners" to "groups" query

Allow filtering by membership.

Changelog: added
---
 app/finders/ci/runners_finder.rb              |  13 +-
 .../resolvers/ci/group_runners_resolver.rb    |  20 ++++
 app/graphql/resolvers/ci/runners_resolver.rb  |  21 +++-
 .../types/ci/runner_membership_filter_enum.rb |  22 ++++
 app/graphql/types/group_type.rb               |   6 +
 doc/api/graphql/reference/index.md            |  31 +++++
 spec/finders/ci/runners_finder_spec.rb        |  30 ++++-
 .../ci/group_runners_resolver_spec.rb         | 112 ++++++++++++++++++
 8 files changed, 244 insertions(+), 11 deletions(-)
 create mode 100644 app/graphql/resolvers/ci/group_runners_resolver.rb
 create mode 100644 app/graphql/types/ci/runner_membership_filter_enum.rb
 create mode 100644 spec/graphql/resolvers/ci/group_runners_resolver_spec.rb

diff --git a/app/finders/ci/runners_finder.rb b/app/finders/ci/runners_finder.rb
index 1ea1c6adb9b039..00ce8dd60d0026 100644
--- a/app/finders/ci/runners_finder.rb
+++ b/app/finders/ci/runners_finder.rb
@@ -49,10 +49,15 @@ def group_runners
 
       raise Gitlab::Access::AccessDeniedError unless can?(@current_user, :admin_group, group)
 
-      # Getting all runners from the group itself and all its descendants
-      descendant_projects = Project.for_group_and_its_subgroups(group)
-
-      @runners = Ci::Runner.belonging_to_group_or_project(group.self_and_descendants, descendant_projects)
+      case @params[:membership]
+      when :direct
+        @runners = Ci::Runner.belonging_to_group(group.id)
+      else
+        # Getting all runners from the group itself and all its descendants
+        descendant_projects = Project.for_group_and_its_subgroups(group)
+
+        @runners = Ci::Runner.belonging_to_group_or_project(group.self_and_descendants, descendant_projects)
+      end
     end
 
     def filter_by_status!
diff --git a/app/graphql/resolvers/ci/group_runners_resolver.rb b/app/graphql/resolvers/ci/group_runners_resolver.rb
new file mode 100644
index 00000000000000..b8b8afeea779ad
--- /dev/null
+++ b/app/graphql/resolvers/ci/group_runners_resolver.rb
@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+module Resolvers
+  module Ci
+    class GroupRunnersResolver < RunnersResolver
+      type Types::Ci::RunnerType.connection_type, null: true
+
+      argument :membership, ::Types::Ci::RunnerMembershipFilterEnum,
+               required: false,
+               default_value: :descendents,
+               description: 'Control which runners to include in the results.'
+
+      protected
+
+      def runners_finder_params(params)
+        super(params).merge(membership: params[:membership])
+      end
+    end
+  end
+end
diff --git a/app/graphql/resolvers/ci/runners_resolver.rb b/app/graphql/resolvers/ci/runners_resolver.rb
index 1957c4ec058dee..d5ba2a3d733d7d 100644
--- a/app/graphql/resolvers/ci/runners_resolver.rb
+++ b/app/graphql/resolvers/ci/runners_resolver.rb
@@ -34,7 +34,7 @@ def resolve_with_lookahead(**args)
             .execute)
       end
 
-      private
+      protected
 
       def runners_finder_params(params)
         {
@@ -47,6 +47,25 @@ def runners_finder_params(params)
             tag_name: node_selection&.selects?(:tag_list)
           }
         }.compact
+         .merge(parent_param)
+      end
+
+      private
+
+      def parent
+        object.respond_to?(:sync) ? object.sync : object
+      end
+
+      def parent_param
+        return {} unless parent
+
+        key = case parent
+              when Group then :group
+              when Project then :project
+              else raise "Unexpected parent type: #{parent.class}"
+              end
+
+        { "#{key}": parent }
       end
     end
   end
diff --git a/app/graphql/types/ci/runner_membership_filter_enum.rb b/app/graphql/types/ci/runner_membership_filter_enum.rb
new file mode 100644
index 00000000000000..7c38b875bce2ea
--- /dev/null
+++ b/app/graphql/types/ci/runner_membership_filter_enum.rb
@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+module Types
+  module Ci
+    class RunnerMembershipFilterEnum < BaseEnum
+      graphql_name 'RunnerMembershipFilter'
+      description 'Values for filtering runners in namespaces.'
+
+      value 'ALL',
+            description: "Include all runners that have either a direct or indirect relationship.",
+            value: :all
+
+      value 'DIRECT',
+            description: "Include runners that have a direct relationship.",
+            value: :direct
+
+      value 'DESCENDENTS',
+            description: "Include runners that have either a direct relationship or a relationship with descendants. These can be project runners or group runners (in the case where group is queried).",
+            value: :descendents
+    end
+  end
+end
diff --git a/app/graphql/types/group_type.rb b/app/graphql/types/group_type.rb
index fbf0084cd0e82e..baf0fa80fc367a 100644
--- a/app/graphql/types/group_type.rb
+++ b/app/graphql/types/group_type.rb
@@ -155,6 +155,12 @@ def label(title:)
           complexity: 5,
           resolver: Resolvers::GroupsResolver
 
+    field :runners, Types::Ci::RunnerType.connection_type,
+          null: true,
+          resolver: Resolvers::Ci::GroupRunnersResolver,
+          description: "Find runners visible to the current user.",
+          feature_flag: :runner_graphql_query
+
     def avatar_url
       object.avatar_url(only_path: false)
     end
diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md
index 6600901e187ba6..b1608fba030ee4 100644
--- a/doc/api/graphql/reference/index.md
+++ b/doc/api/graphql/reference/index.md
@@ -10004,6 +10004,27 @@ four standard [pagination arguments](#connection-pagination-arguments):
 | <a id="groupprojectssearch"></a>`search` | [`String`](#string) | Search project with most similar names or paths. |
 | <a id="groupprojectssort"></a>`sort` | [`NamespaceProjectSort`](#namespaceprojectsort) | Sort projects by this criteria. |
 
+##### `Group.runners`
+
+Find runners visible to the current user. Available only when feature flag `runner_graphql_query` is enabled. This flag is enabled by default.
+
+Returns [`CiRunnerConnection`](#cirunnerconnection).
+
+This field returns a [connection](#connections). It accepts the
+four standard [pagination arguments](#connection-pagination-arguments):
+`before: String`, `after: String`, `first: Int`, `last: Int`.
+
+###### Arguments
+
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| <a id="grouprunnersmembership"></a>`membership` | [`RunnerMembershipFilter`](#runnermembershipfilter) | Control which runners to include in the results. |
+| <a id="grouprunnerssearch"></a>`search` | [`String`](#string) | Filter by full token or partial text in description field. |
+| <a id="grouprunnerssort"></a>`sort` | [`CiRunnerSort`](#cirunnersort) | Sort order of results. |
+| <a id="grouprunnersstatus"></a>`status` | [`CiRunnerStatus`](#cirunnerstatus) | Filter runners by status. |
+| <a id="grouprunnerstaglist"></a>`tagList` | [`[String!]`](#string) | Filter by tags associated with the runner (comma-separated or array). |
+| <a id="grouprunnerstype"></a>`type` | [`CiRunnerType`](#cirunnertype) | Filter runners by type. |
+
 ##### `Group.timelogs`
 
 Time logged on issues and merge requests in the group and its subgroups.
@@ -15626,6 +15647,16 @@ Status of a requirement based on last test report.
 | <a id="requirementstatusfiltermissing"></a>`MISSING` | Requirements without any test report. |
 | <a id="requirementstatusfilterpassed"></a>`PASSED` |  |
 
+### `RunnerMembershipFilter`
+
+Values for filtering runners in namespaces.
+
+| Value | Description |
+| ----- | ----------- |
+| <a id="runnermembershipfilterall"></a>`ALL` | Include all runners that have either a direct or indirect relationship. |
+| <a id="runnermembershipfilterdescendents"></a>`DESCENDENTS` | Include runners that have either a direct relationship or a relationship with descendants. These can be project runners or group runners (in the case where group is queried). |
+| <a id="runnermembershipfilterdirect"></a>`DIRECT` | Include runners that have a direct relationship. |
+
 ### `SastUiComponentSize`
 
 Size of UI component in SAST configuration page.
diff --git a/spec/finders/ci/runners_finder_spec.rb b/spec/finders/ci/runners_finder_spec.rb
index a20cd5c681d96f..eca632600036d3 100644
--- a/spec/finders/ci/runners_finder_spec.rb
+++ b/spec/finders/ci/runners_finder_spec.rb
@@ -182,7 +182,18 @@
     end
 
     describe '#execute' do
-      subject { described_class.new(current_user: user, params: params.merge(group: group)).execute }
+      let(:group_params) { { group: group } }
+
+      subject { described_class.new(current_user: user, params: params.merge(group_params)).execute }
+
+      shared_examples 'membership equal to :descendants' do
+        it 'returns all 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])
+        end
+      end
 
       context 'with user as group owner' do
         before do
@@ -190,11 +201,18 @@
         end
 
         context 'passing no params' 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])
+          it_behaves_like 'membership equal to :descendants'
+        end
+
+        context 'with :descendants membership' do
+          it_behaves_like 'membership equal to :descendants'
+        end
+
+        context 'with :direct membership' do
+          let(:group_params) { { group: group, membership: :direct } }
+
+          it 'returns runners belonging to group' do
+            expect(subject).to eq([runner_group])
           end
         end
 
diff --git a/spec/graphql/resolvers/ci/group_runners_resolver_spec.rb b/spec/graphql/resolvers/ci/group_runners_resolver_spec.rb
new file mode 100644
index 00000000000000..3cb4a01c59d951
--- /dev/null
+++ b/spec/graphql/resolvers/ci/group_runners_resolver_spec.rb
@@ -0,0 +1,112 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Resolvers::Ci::GroupRunnersResolver do
+  include GraphqlHelpers
+
+  include_context 'runners resolver setup'
+
+  it_behaves_like Resolvers::Ci::RunnersResolver
+
+  shared_context 'resolve args with membership' do
+    let(:membership) { nil }
+    let(:args) do
+      { membership: membership }.reject { |_, v| v.nil? }
+    end
+  end
+
+  describe '#resolve' do
+    subject { resolve(described_class, obj: obj, ctx: { current_user: user }, args: args).items.to_a }
+
+    context 'with user as group owner' do
+      before do
+        group.add_owner(user)
+      end
+
+      context 'with obj set to subgroup' do
+        let(:obj) { subgroup }
+
+        context 'with a membership argument' do
+          include_context 'resolve args with membership'
+
+          context 'set to :all' do
+            let(:membership) { :all }
+
+            it { is_expected.to contain_exactly(subgroup_runner) }
+          end
+
+          context 'set to :direct' do
+            let(:membership) { :direct }
+
+            it { is_expected.to contain_exactly(subgroup_runner) }
+          end
+
+          context 'not set' do
+            it { is_expected.to contain_exactly(subgroup_runner) }
+          end
+
+          context 'set to :descendants' do
+            let(:membership) { :descendants }
+
+            it { is_expected.to contain_exactly(subgroup_runner) }
+          end
+        end
+      end
+
+      context 'with obj set to group' do
+        let(:obj) { group }
+
+        context 'with a membership argument' do
+          include_context 'resolve args with membership'
+
+          shared_examples 'returns self and descendant runners' do
+            it { is_expected.to contain_exactly(group_runner, subgroup_runner, offline_project_runner, inactive_project_runner) }
+          end
+
+          context 'set to :all' do
+            let(:membership) { :all }
+
+            include_examples 'returns self and descendant runners'
+          end
+
+          context 'set to :direct' do
+            let(:membership) { :direct }
+
+            it { is_expected.to contain_exactly(group_runner) }
+          end
+
+          context 'not set' do
+            include_examples 'returns self and descendant runners'
+          end
+
+          context 'set to :descendants' do
+            let(:membership) { :descendants }
+
+            include_examples 'returns self and descendant runners'
+          end
+        end
+      end
+    end
+
+    context 'with obj set to subgroup' do
+      let(:obj) { subgroup }
+
+      context 'with a membership argument' do
+        include_context 'resolve args with membership'
+
+        context 'set to :direct' do
+          let(:membership) { :direct }
+
+          it { is_expected.to eq([]) }
+        end
+
+        context 'set to :descendants' do
+          let(:membership) { :descendants }
+
+          it { is_expected.to eq([]) }
+        end
+      end
+    end
+  end
+end
-- 
GitLab


From f0d2c99b20dc8ec7e0006fc3c3e9b693fa233d30 Mon Sep 17 00:00:00 2001
From: Pedro Pombeiro <noreply@pedro.pombei.ro>
Date: Mon, 26 Jul 2021 19:28:06 +0200
Subject: [PATCH 04/10] Address MR review comments

---
 app/finders/ci/runners_finder.rb              | 28 +++++++++----------
 .../resolvers/ci/group_runners_resolver.rb    |  2 +-
 .../types/ci/runner_membership_filter_enum.rb |  8 ++----
 doc/api/graphql/reference/index.md            |  3 +-
 spec/finders/ci/runners_finder_spec.rb        | 19 ++++++++++++-
 .../ci/group_runners_resolver_spec.rb         | 12 --------
 6 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/app/finders/ci/runners_finder.rb b/app/finders/ci/runners_finder.rb
index 00ce8dd60d0026..8bc2a47a024fd9 100644
--- a/app/finders/ci/runners_finder.rb
+++ b/app/finders/ci/runners_finder.rb
@@ -9,6 +9,7 @@ class RunnersFinder < UnionFinder
 
     def initialize(current_user:, params:)
       @params = params
+      @group = params.delete(:group)
       @current_user = current_user
     end
 
@@ -33,7 +34,7 @@ def sort_key
     private
 
     def search!
-      @params.has_key?(:group) ? group_runners : all_runners
+      @group ? group_runners : all_runners
 
       @runners = @runners.search(@params[:search]) if @params[:search].present?
     end
@@ -45,19 +46,18 @@ def all_runners
     end
 
     def group_runners
-      group = @params[:group]
-
-      raise Gitlab::Access::AccessDeniedError unless can?(@current_user, :admin_group, group)
-
-      case @params[:membership]
-      when :direct
-        @runners = Ci::Runner.belonging_to_group(group.id)
-      else
-        # Getting all runners from the group itself and all its descendants
-        descendant_projects = Project.for_group_and_its_subgroups(group)
-
-        @runners = Ci::Runner.belonging_to_group_or_project(group.self_and_descendants, descendant_projects)
-      end
+      raise Gitlab::Access::AccessDeniedError unless can?(@current_user, :admin_group, @group)
+
+      @runners = case @params[:membership]
+                 when :direct
+                   Ci::Runner.belonging_to_group(@group.id)
+                 when :descendants, nil
+                   # Getting all runners from the group itself and all its descendant groups/projects
+                   descendant_projects = Project.for_group_and_its_subgroups(@group)
+                   Ci::Runner.belonging_to_group_or_project(@group.self_and_descendants, descendant_projects)
+                 else
+                   raise ArgumentError, 'Invalid membership filter'
+                 end
     end
 
     def filter_by_status!
diff --git a/app/graphql/resolvers/ci/group_runners_resolver.rb b/app/graphql/resolvers/ci/group_runners_resolver.rb
index b8b8afeea779ad..0f28d49e739f43 100644
--- a/app/graphql/resolvers/ci/group_runners_resolver.rb
+++ b/app/graphql/resolvers/ci/group_runners_resolver.rb
@@ -7,7 +7,7 @@ class GroupRunnersResolver < RunnersResolver
 
       argument :membership, ::Types::Ci::RunnerMembershipFilterEnum,
                required: false,
-               default_value: :descendents,
+               default_value: :descendants,
                description: 'Control which runners to include in the results.'
 
       protected
diff --git a/app/graphql/types/ci/runner_membership_filter_enum.rb b/app/graphql/types/ci/runner_membership_filter_enum.rb
index 7c38b875bce2ea..2e1051b21518ba 100644
--- a/app/graphql/types/ci/runner_membership_filter_enum.rb
+++ b/app/graphql/types/ci/runner_membership_filter_enum.rb
@@ -6,17 +6,13 @@ class RunnerMembershipFilterEnum < BaseEnum
       graphql_name 'RunnerMembershipFilter'
       description 'Values for filtering runners in namespaces.'
 
-      value 'ALL',
-            description: "Include all runners that have either a direct or indirect relationship.",
-            value: :all
-
       value 'DIRECT',
             description: "Include runners that have a direct relationship.",
             value: :direct
 
-      value 'DESCENDENTS',
+      value 'DESCENDANTS',
             description: "Include runners that have either a direct relationship or a relationship with descendants. These can be project runners or group runners (in the case where group is queried).",
-            value: :descendents
+            value: :descendants
     end
   end
 end
diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md
index b1608fba030ee4..a9c2a7b522c487 100644
--- a/doc/api/graphql/reference/index.md
+++ b/doc/api/graphql/reference/index.md
@@ -15653,8 +15653,7 @@ Values for filtering runners in namespaces.
 
 | Value | Description |
 | ----- | ----------- |
-| <a id="runnermembershipfilterall"></a>`ALL` | Include all runners that have either a direct or indirect relationship. |
-| <a id="runnermembershipfilterdescendents"></a>`DESCENDENTS` | Include runners that have either a direct relationship or a relationship with descendants. These can be project runners or group runners (in the case where group is queried). |
+| <a id="runnermembershipfilterdescendants"></a>`DESCENDANTS` | Include runners that have either a direct relationship or a relationship with descendants. These can be project runners or group runners (in the case where group is queried). |
 | <a id="runnermembershipfilterdirect"></a>`DIRECT` | Include runners that have a direct relationship. |
 
 ### `SastUiComponentSize`
diff --git a/spec/finders/ci/runners_finder_spec.rb b/spec/finders/ci/runners_finder_spec.rb
index eca632600036d3..02049fe1f7695e 100644
--- a/spec/finders/ci/runners_finder_spec.rb
+++ b/spec/finders/ci/runners_finder_spec.rb
@@ -18,6 +18,13 @@
           end
         end
 
+        context 'with nil group' do
+          it 'returns all runners' do
+            expect(Ci::Runner).to receive(:with_tags).and_call_original
+            expect(described_class.new(current_user: admin, params: { group: nil }).execute).to match_array [runner1, runner2]
+          end
+        end
+
         context 'with preload param set to :tag_name true' do
           it 'requests tags' do
             expect(Ci::Runner).to receive(:with_tags).and_call_original
@@ -158,6 +165,7 @@
     let_it_be(:project_4) { create(:project, group: sub_group_2) }
     let_it_be(:project_5) { create(:project, group: sub_group_3) }
     let_it_be(:project_6) { create(:project, group: sub_group_4) }
+    let_it_be(:runner_instance) { create(:ci_runner, :instance, contacted_at: 13.minutes.ago) }
     let_it_be(:runner_group) { create(:ci_runner, :group, contacted_at: 12.minutes.ago) }
     let_it_be(:runner_sub_group_1) { create(:ci_runner, :group, active: false, contacted_at: 11.minutes.ago) }
     let_it_be(:runner_sub_group_2) { create(:ci_runner, :group, contacted_at: 10.minutes.ago) }
@@ -187,7 +195,7 @@
       subject { described_class.new(current_user: user, params: params.merge(group_params)).execute }
 
       shared_examples 'membership equal to :descendants' do
-        it 'returns all runners' 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,
@@ -216,6 +224,15 @@
           end
         end
 
+        context 'with nil group' do
+          let(:group_params) { { group: nil } }
+
+          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([])
+          end
+        end
+
         context 'with sort param' do
           let(:params) { { sort: 'contacted_asc' } }
 
diff --git a/spec/graphql/resolvers/ci/group_runners_resolver_spec.rb b/spec/graphql/resolvers/ci/group_runners_resolver_spec.rb
index 3cb4a01c59d951..224e057673b3e7 100644
--- a/spec/graphql/resolvers/ci/group_runners_resolver_spec.rb
+++ b/spec/graphql/resolvers/ci/group_runners_resolver_spec.rb
@@ -30,12 +30,6 @@
         context 'with a membership argument' do
           include_context 'resolve args with membership'
 
-          context 'set to :all' do
-            let(:membership) { :all }
-
-            it { is_expected.to contain_exactly(subgroup_runner) }
-          end
-
           context 'set to :direct' do
             let(:membership) { :direct }
 
@@ -64,12 +58,6 @@
             it { is_expected.to contain_exactly(group_runner, subgroup_runner, offline_project_runner, inactive_project_runner) }
           end
 
-          context 'set to :all' do
-            let(:membership) { :all }
-
-            include_examples 'returns self and descendant runners'
-          end
-
           context 'set to :direct' do
             let(:membership) { :direct }
 
-- 
GitLab


From 1cf6937975f9b0c20f93e0c0fd840156cf25a162 Mon Sep 17 00:00:00 2001
From: Pedro Pombeiro <noreply@pedro.pombei.ro>
Date: Tue, 27 Jul 2021 12:47:26 +0200
Subject: [PATCH 05/10] Define target_group and membership in RunnersFinder
 specs

---
 spec/finders/ci/runners_finder_spec.rb | 109 +++++++++++++------------
 1 file changed, 58 insertions(+), 51 deletions(-)

diff --git a/spec/finders/ci/runners_finder_spec.rb b/spec/finders/ci/runners_finder_spec.rb
index 02049fe1f7695e..1bf98cd18430df 100644
--- a/spec/finders/ci/runners_finder_spec.rb
+++ b/spec/finders/ci/runners_finder_spec.rb
@@ -179,7 +179,10 @@
     let_it_be(:runner_project_6) { create(:ci_runner, :project, contacted_at: 2.minutes.ago, projects: [project_5])}
     let_it_be(:runner_project_7) { create(:ci_runner, :project, contacted_at: 1.minute.ago, projects: [project_6])}
 
-    let(:params) { {} }
+    let(:target_group) { nil }
+    let(:membership) { nil }
+    let(:extra_params) { {} }
+    let(:params) { { group: target_group, membership: membership }.merge(extra_params).reject { |_, v| v.nil? } }
 
     before do
       group.runners << runner_group
@@ -190,9 +193,7 @@
     end
 
     describe '#execute' do
-      let(:group_params) { { group: group } }
-
-      subject { described_class.new(current_user: user, params: params.merge(group_params)).execute }
+      subject { described_class.new(current_user: user, params: params).execute }
 
       shared_examples 'membership equal to :descendants' do
         it 'returns all descendant runners' do
@@ -208,74 +209,80 @@
           group.add_owner(user)
         end
 
-        context 'passing no params' do
-          it_behaves_like 'membership equal to :descendants'
-        end
+        context 'with :group as target group' do
+          let(:target_group) { group }
 
-        context 'with :descendants membership' do
-          it_behaves_like 'membership equal to :descendants'
-        end
+          context 'passing no params' do
+            it_behaves_like 'membership equal to :descendants'
+          end
 
-        context 'with :direct membership' do
-          let(:group_params) { { group: group, 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 nil group' do
-          let(:group_params) { { group: nil } }
+          context 'with :direct membership' do
+            let(:membership) { :direct }
 
-          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 'returns runners belonging to group' do
+              expect(subject).to eq([runner_group])
+            end
           end
-        end
 
-        context 'with sort param' do
-          let(: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 'returns no runners' do
+              # Query should run against all runners, however since user is not admin, query returns no results
+              expect(subject).to eq([])
+            end
           end
-        end
 
-        context 'filtering' do
-          context 'by search term' do
-            let(:params) { { search: 'runner_project_search' } }
+          context 'with sort param' do
+            let(:extra_params) { { sort: 'contacted_asc' } }
 
-            it 'returns correct runner' do
-              expect(subject).to eq([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 status' do
-            let(:params) { { status_status: 'paused' } }
+          context 'filtering' do
+            context 'by search term' do
+              let(:extra_params) { { search: 'runner_project_search' } }
 
-            it 'returns correct runner' do
-              expect(subject).to eq([runner_sub_group_1])
+              it 'returns correct runner' do
+                expect(subject).to eq([runner_project_3])
+              end
             end
-          end
 
-          context 'by tag_name' do
-            let(:params) { { tag_name: %w[runner_tag] } }
+            context 'by status' do
+              let(:extra_params) { { status_status: 'paused' } }
 
-            it 'returns correct runner' do
-              expect(subject).to eq([runner_project_5])
+              it 'returns correct runner' do
+                expect(subject).to eq([runner_sub_group_1])
+              end
+            end
+
+            context 'by tag_name' do
+              let(:extra_params) { { tag_name: %w[runner_tag] } }
+
+              it 'returns correct runner' do
+                expect(subject).to eq([runner_project_5])
+              end
             end
-          end
 
-          context 'by runner type' do
-            let(:params) { { type_type: 'project_type' } }
+            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])
+              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
@@ -322,7 +329,7 @@
       end
 
       context 'with params' do
-        let(:params) { { sort: 'contacted_asc' } }
+        let(:extra_params) { { sort: 'contacted_asc' } }
 
         it 'returns contacted_asc' do
           expect(subject).to eq('contacted_asc')
-- 
GitLab


From 595f5270481a9178e462b429cf636a7dd3b41774 Mon Sep 17 00:00:00 2001
From: Pedro Pombeiro <noreply@pedro.pombei.ro>
Date: Wed, 28 Jul 2021 16:00:48 +0200
Subject: [PATCH 06/10] Address MR review comments

---
 .../resolvers/ci/group_runners_resolver.rb    |  6 ++++++
 app/graphql/resolvers/ci/runners_resolver.rb  | 18 ++++++-----------
 .../ci/group_runners_resolver_spec.rb         | 16 +++++++++++++++
 .../resolvers/ci/runners_resolver_spec.rb     | 20 +++++++++++++++++++
 4 files changed, 48 insertions(+), 12 deletions(-)

diff --git a/app/graphql/resolvers/ci/group_runners_resolver.rb b/app/graphql/resolvers/ci/group_runners_resolver.rb
index 0f28d49e739f43..e9c399d3855bb5 100644
--- a/app/graphql/resolvers/ci/group_runners_resolver.rb
+++ b/app/graphql/resolvers/ci/group_runners_resolver.rb
@@ -15,6 +15,12 @@ class GroupRunnersResolver < RunnersResolver
       def runners_finder_params(params)
         super(params).merge(membership: params[:membership])
       end
+
+      def parent_param
+        raise 'Expected group missing' unless parent.is_a?(Group)
+
+        { group: parent }
+      end
     end
   end
 end
diff --git a/app/graphql/resolvers/ci/runners_resolver.rb b/app/graphql/resolvers/ci/runners_resolver.rb
index d5ba2a3d733d7d..07105701daa3e3 100644
--- a/app/graphql/resolvers/ci/runners_resolver.rb
+++ b/app/graphql/resolvers/ci/runners_resolver.rb
@@ -50,22 +50,16 @@ def runners_finder_params(params)
          .merge(parent_param)
       end
 
-      private
-
-      def parent
-        object.respond_to?(:sync) ? object.sync : object
-      end
-
       def parent_param
         return {} unless parent
 
-        key = case parent
-              when Group then :group
-              when Project then :project
-              else raise "Unexpected parent type: #{parent.class}"
-              end
+        raise "Unexpected parent type: #{parent.class}"
+      end
+
+      private
 
-        { "#{key}": parent }
+      def parent
+        object.respond_to?(:sync) ? object.sync : object
       end
     end
   end
diff --git a/spec/graphql/resolvers/ci/group_runners_resolver_spec.rb b/spec/graphql/resolvers/ci/group_runners_resolver_spec.rb
index 224e057673b3e7..1af1cf9661f47e 100644
--- a/spec/graphql/resolvers/ci/group_runners_resolver_spec.rb
+++ b/spec/graphql/resolvers/ci/group_runners_resolver_spec.rb
@@ -24,6 +24,22 @@
         group.add_owner(user)
       end
 
+      context 'with obj set to nil' do
+        let(:obj) { nil }
+
+        context 'with a membership argument' do
+          include_context 'resolve args with membership'
+
+          context 'set to :direct' do
+            let(:membership) { :direct }
+
+            it 'raises an error' do
+              expect { subject }.to raise_error('Expected group missing')
+            end
+          end
+        end
+      end
+
       context 'with obj set to subgroup' do
         let(:obj) { subgroup }
 
diff --git a/spec/graphql/resolvers/ci/runners_resolver_spec.rb b/spec/graphql/resolvers/ci/runners_resolver_spec.rb
index 588ca82aa2fe50..6ecd2c0af8f227 100644
--- a/spec/graphql/resolvers/ci/runners_resolver_spec.rb
+++ b/spec/graphql/resolvers/ci/runners_resolver_spec.rb
@@ -8,4 +8,24 @@
   include_context 'runners resolver setup'
 
   it_behaves_like Resolvers::Ci::RunnersResolver
+
+  describe '#resolve' do
+    subject { resolve(described_class, obj: obj, ctx: { current_user: user }, args: {}) }
+
+    context 'with obj set to nil value' do
+      let(:obj) { nil }
+
+      it 'does not raise an error' do
+        expect { subject }.not_to raise_error
+      end
+    end
+
+    context 'with obj set to unsupported value' do
+      let_it_be(:obj) { build(:group) }
+
+      it 'returns no runners' do
+        expect { subject }.to raise_error("Unexpected parent type: #{obj.class}")
+      end
+    end
+  end
 end
-- 
GitLab


From 6c54ee116e5592b108f52075eab3d402944ffa45 Mon Sep 17 00:00:00 2001
From: Pedro Pombeiro <noreply@pedro.pombei.ro>
Date: Wed, 28 Jul 2021 16:10:18 +0200
Subject: [PATCH 07/10] Address MR review comments

---
 spec/finders/ci/runners_finder_spec.rb        |  8 +++++++
 .../ci/group_runners_resolver_spec.rb         | 24 ++++++++++---------
 .../runners_resolver_shared_examples.rb       |  2 +-
 3 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/spec/finders/ci/runners_finder_spec.rb b/spec/finders/ci/runners_finder_spec.rb
index 1bf98cd18430df..10d3f641e02bf6 100644
--- a/spec/finders/ci/runners_finder_spec.rb
+++ b/spec/finders/ci/runners_finder_spec.rb
@@ -230,6 +230,14 @@
             end
           end
 
+          context 'with unknown membership' do
+            let(:membership) { :unsupported }
+
+            it 'raises an error' do
+              expect { subject }.to raise_error(ArgumentError, 'Invalid membership filter')
+            end
+          end
+
           context 'with nil group' do
             let(:target_group) { nil }
 
diff --git a/spec/graphql/resolvers/ci/group_runners_resolver_spec.rb b/spec/graphql/resolvers/ci/group_runners_resolver_spec.rb
index 1af1cf9661f47e..0e2a544e045195 100644
--- a/spec/graphql/resolvers/ci/group_runners_resolver_spec.rb
+++ b/spec/graphql/resolvers/ci/group_runners_resolver_spec.rb
@@ -93,22 +93,24 @@
       end
     end
 
-    context 'with obj set to subgroup' do
-      let(:obj) { subgroup }
+    context 'when user has no access to the group' do
+      context 'with obj set to subgroup' do
+        let(:obj) { subgroup }
 
-      context 'with a membership argument' do
-        include_context 'resolve args with membership'
+        context 'with a membership argument' do
+          include_context 'resolve args with membership'
 
-        context 'set to :direct' do
-          let(:membership) { :direct }
+          context 'set to :direct' do
+            let(:membership) { :direct }
 
-          it { is_expected.to eq([]) }
-        end
+            it { is_expected.to eq([]) }
+          end
 
-        context 'set to :descendants' do
-          let(:membership) { :descendants }
+          context 'set to :descendants' do
+            let(:membership) { :descendants }
 
-          it { is_expected.to eq([]) }
+            it { is_expected.to eq([]) }
+          end
         end
       end
     end
diff --git a/spec/support/shared_examples/graphql/resolvers/runners_resolver_shared_examples.rb b/spec/support/shared_examples/graphql/resolvers/runners_resolver_shared_examples.rb
index c89d1713e03b33..d5f30e3aba94a0 100644
--- a/spec/support/shared_examples/graphql/resolvers/runners_resolver_shared_examples.rb
+++ b/spec/support/shared_examples/graphql/resolvers/runners_resolver_shared_examples.rb
@@ -12,7 +12,7 @@
       end
 
       context 'when the user cannot see runners' do
-        let(:user) { create(:user) }
+        let(:user) { build(:user) }
 
         it 'returns no runners' do
           is_expected.to be_empty
-- 
GitLab


From f04543524730b4bf3cebd20acb672c431df7a0af Mon Sep 17 00:00:00 2001
From: Pedro Pombeiro <noreply@pedro.pombei.ro>
Date: Wed, 28 Jul 2021 17:50:48 +0200
Subject: [PATCH 08/10] Fix GroupRunnersResolver specs

---
 .../ci/group_runners_resolver_spec.rb         | 178 ++++++++++++++++-
 .../resolvers/ci/runners_resolver_spec.rb     | 180 ++++++++++++++++--
 .../runners_resolver_shared_examples.rb       | 175 -----------------
 3 files changed, 345 insertions(+), 188 deletions(-)
 delete mode 100644 spec/support/shared_examples/graphql/resolvers/runners_resolver_shared_examples.rb

diff --git a/spec/graphql/resolvers/ci/group_runners_resolver_spec.rb b/spec/graphql/resolvers/ci/group_runners_resolver_spec.rb
index 0e2a544e045195..c5363d78e1d037 100644
--- a/spec/graphql/resolvers/ci/group_runners_resolver_spec.rb
+++ b/spec/graphql/resolvers/ci/group_runners_resolver_spec.rb
@@ -7,8 +7,6 @@
 
   include_context 'runners resolver setup'
 
-  it_behaves_like Resolvers::Ci::RunnersResolver
-
   shared_context 'resolve args with membership' do
     let(:membership) { nil }
     let(:args) do
@@ -17,6 +15,8 @@
   end
 
   describe '#resolve' do
+    let(:args) { }
+
     subject { resolve(described_class, obj: obj, ctx: { current_user: user }, args: args).items.to_a }
 
     context 'with user as group owner' do
@@ -64,9 +64,183 @@
         end
       end
 
+      context 'with obj set to unsupported value' do
+        let_it_be(:obj) { build(:project) }
+
+        it 'raises error' do
+          expect { subject }.to raise_error('Expected group missing')
+        end
+      end
+
       context 'with obj set to group' do
         let(:obj) { group }
 
+        context 'with empty args' do
+          let(:args) do
+            {}
+          end
+
+          context 'when the user cannot see runners' do
+            let(:user) { build(:user) }
+
+            it 'returns no runners' do
+              is_expected.to be_empty
+            end
+          end
+
+          context 'without sort' do
+            it 'returns all the runners' do
+              is_expected.to contain_exactly(inactive_project_runner, offline_project_runner, group_runner, subgroup_runner)
+            end
+          end
+        end
+
+        context 'with a sort argument' do
+          context "set to :contacted_asc" do
+            let(:args) do
+              { sort: :contacted_asc }
+            end
+
+            it { is_expected.to eq([offline_project_runner, inactive_project_runner, group_runner, subgroup_runner]) }
+          end
+
+          context "set to :contacted_desc" do
+            let(:args) do
+              { sort: :contacted_desc }
+            end
+
+            it { is_expected.to eq([offline_project_runner, inactive_project_runner, group_runner, subgroup_runner].reverse) }
+          end
+
+          context "set to :created_at_desc" do
+            let(:args) do
+              { sort: :created_at_desc }
+            end
+
+            it { is_expected.to eq([subgroup_runner, group_runner, offline_project_runner, inactive_project_runner]) }
+          end
+
+          context "set to :created_at_asc" do
+            let(:args) do
+              { sort: :created_at_asc }
+            end
+
+            it { is_expected.to eq([subgroup_runner, group_runner, offline_project_runner, inactive_project_runner].reverse) }
+          end
+        end
+
+        context 'when type is filtered' do
+          let(:args) do
+            { type: runner_type.to_s }
+          end
+
+          context 'to instance runners' do
+            let(:runner_type) { :instance_type }
+
+            it 'returns empty array' do
+              is_expected.to eq([])
+            end
+          end
+
+          context 'to group runners' do
+            let(:runner_type) { :group_type }
+
+            it 'returns the group runner' do
+              is_expected.to contain_exactly(group_runner, subgroup_runner)
+            end
+          end
+
+          context 'to project runners' do
+            let(:runner_type) { :project_type }
+
+            it 'returns the project runner' do
+              is_expected.to contain_exactly(inactive_project_runner, offline_project_runner)
+            end
+          end
+        end
+
+        context 'when status is filtered' do
+          let(:args) do
+            { status: runner_status.to_s }
+          end
+
+          context 'to active runners' do
+            let(:runner_status) { :active }
+
+            it 'returns the instance and group runners' do
+              is_expected.to contain_exactly(offline_project_runner, group_runner, subgroup_runner)
+            end
+          end
+
+          context 'to offline runners' do
+            let(:runner_status) { :offline }
+
+            it 'returns the offline project runner' do
+              is_expected.to contain_exactly(offline_project_runner)
+            end
+          end
+        end
+
+        context 'when tag list is filtered' do
+          let(:args) do
+            { tag_list: tag_list }
+          end
+
+          context 'with "project_runner" tag' do
+            let(:tag_list) { ['project_runner'] }
+
+            it 'returns the project_runner runners' do
+              is_expected.to contain_exactly(offline_project_runner, inactive_project_runner)
+            end
+          end
+
+          context 'with "project_runner" and "active_runner" tags as comma-separated string' do
+            let(:tag_list) { ['project_runner,active_runner'] }
+
+            it 'returns the offline_project_runner runner' do
+              is_expected.to contain_exactly(offline_project_runner)
+            end
+          end
+
+          context 'with "active_runner" and "project_runner" tags as array' do
+            let(:tag_list) { %w[project_runner active_runner] }
+
+            it 'returns the offline_project_runner runner' do
+              is_expected.to contain_exactly(offline_project_runner)
+            end
+          end
+        end
+
+        context 'when text is filtered' do
+          let(:args) do
+            { search: search_term }
+          end
+
+          context 'to "project"' do
+            let(:search_term) { 'project' }
+
+            it 'returns both project runners' do
+              is_expected.to contain_exactly(inactive_project_runner, offline_project_runner)
+            end
+          end
+
+          context 'to "group"' do
+            let(:search_term) { 'group' }
+
+            it 'returns group runners' do
+              is_expected.to contain_exactly(group_runner, subgroup_runner)
+            end
+          end
+
+          context 'to "defghi"' do
+            let(:search_term) { 'defghi' }
+
+            it 'returns runners containing term in token' do
+              is_expected.to contain_exactly(offline_project_runner)
+            end
+          end
+        end
+
         context 'with a membership argument' do
           include_context 'resolve args with membership'
 
diff --git a/spec/graphql/resolvers/ci/runners_resolver_spec.rb b/spec/graphql/resolvers/ci/runners_resolver_spec.rb
index 6ecd2c0af8f227..bbaf08c9628a0f 100644
--- a/spec/graphql/resolvers/ci/runners_resolver_spec.rb
+++ b/spec/graphql/resolvers/ci/runners_resolver_spec.rb
@@ -7,24 +7,182 @@
 
   include_context 'runners resolver setup'
 
-  it_behaves_like Resolvers::Ci::RunnersResolver
-
   describe '#resolve' do
-    subject { resolve(described_class, obj: obj, ctx: { current_user: user }, args: {}) }
+    let(:obj) { nil }
+
+    subject { resolve(described_class, obj: obj, ctx: { current_user: user }, args: args).items.to_a }
+
+    context 'with empty args' do
+      let(:args) do
+        {}
+      end
+
+      context 'with obj set to unsupported value' do
+        let_it_be(:obj) { build(:group) }
+
+        it 'raises error' do
+          expect { subject }.to raise_error("Unexpected parent type: #{obj.class}")
+        end
+      end
+
+      context 'when the user cannot see runners' do
+        let(:user) { build(:user) }
+
+        it 'returns no runners' do
+          is_expected.to be_empty
+        end
+      end
+
+      context 'without sort' do
+        it 'returns all the runners' do
+          is_expected.to contain_exactly(inactive_project_runner, offline_project_runner, group_runner, subgroup_runner, instance_runner)
+        end
+      end
+    end
+
+    context 'with a sort argument' do
+      context "set to :contacted_asc" do
+        let(:args) do
+          { sort: :contacted_asc }
+        end
+
+        it { is_expected.to eq([offline_project_runner, instance_runner, inactive_project_runner, group_runner, subgroup_runner]) }
+      end
 
-    context 'with obj set to nil value' do
-      let(:obj) { nil }
+      context "set to :contacted_desc" do
+        let(:args) do
+          { sort: :contacted_desc }
+        end
 
-      it 'does not raise an error' do
-        expect { subject }.not_to raise_error
+        it { is_expected.to eq([offline_project_runner, instance_runner, inactive_project_runner, group_runner, subgroup_runner].reverse) }
+      end
+
+      context "set to :created_at_desc" do
+        let(:args) do
+          { sort: :created_at_desc }
+        end
+
+        it { is_expected.to eq([instance_runner, subgroup_runner, group_runner, offline_project_runner, inactive_project_runner]) }
+      end
+
+      context "set to :created_at_asc" do
+        let(:args) do
+          { sort: :created_at_asc }
+        end
+
+        it { is_expected.to eq([instance_runner, subgroup_runner, group_runner, offline_project_runner, inactive_project_runner].reverse) }
       end
     end
 
-    context 'with obj set to unsupported value' do
-      let_it_be(:obj) { build(:group) }
+    context 'when type is filtered' do
+      let(:args) do
+        { type: runner_type.to_s }
+      end
+
+      context 'to instance runners' do
+        let(:runner_type) { :instance_type }
+
+        it 'returns the instance runner' do
+          is_expected.to contain_exactly(instance_runner)
+        end
+      end
+
+      context 'to group runners' do
+        let(:runner_type) { :group_type }
+
+        it 'returns the group runner' do
+          is_expected.to contain_exactly(group_runner, subgroup_runner)
+        end
+      end
+
+      context 'to project runners' do
+        let(:runner_type) { :project_type }
+
+        it 'returns the project runner' do
+          is_expected.to contain_exactly(inactive_project_runner, offline_project_runner)
+        end
+      end
+    end
+
+    context 'when status is filtered' do
+      let(:args) do
+        { status: runner_status.to_s }
+      end
+
+      context 'to active runners' do
+        let(:runner_status) { :active }
+
+        it 'returns the instance and group runners' do
+          is_expected.to contain_exactly(offline_project_runner, group_runner, subgroup_runner, instance_runner)
+        end
+      end
+
+      context 'to offline runners' do
+        let(:runner_status) { :offline }
+
+        it 'returns the offline project runner' do
+          is_expected.to contain_exactly(offline_project_runner)
+        end
+      end
+    end
+
+    context 'when tag list is filtered' do
+      let(:args) do
+        { tag_list: tag_list }
+      end
+
+      context 'with "project_runner" tag' do
+        let(:tag_list) { ['project_runner'] }
+
+        it 'returns the project_runner runners' do
+          is_expected.to contain_exactly(offline_project_runner, inactive_project_runner)
+        end
+      end
+
+      context 'with "project_runner" and "active_runner" tags as comma-separated string' do
+        let(:tag_list) { ['project_runner,active_runner'] }
+
+        it 'returns the offline_project_runner runner' do
+          is_expected.to contain_exactly(offline_project_runner)
+        end
+      end
+
+      context 'with "active_runner" and "instance_runner" tags as array' do
+        let(:tag_list) { %w[instance_runner active_runner] }
+
+        it 'returns the instance_runner runner' do
+          is_expected.to contain_exactly(instance_runner)
+        end
+      end
+    end
+
+    context 'when text is filtered' do
+      let(:args) do
+        { search: search_term }
+      end
+
+      context 'to "project"' do
+        let(:search_term) { 'project' }
+
+        it 'returns both project runners' do
+          is_expected.to contain_exactly(inactive_project_runner, offline_project_runner)
+        end
+      end
+
+      context 'to "group"' do
+        let(:search_term) { 'group' }
+
+        it 'returns group runners' do
+          is_expected.to contain_exactly(group_runner, subgroup_runner)
+        end
+      end
+
+      context 'to "defghi"' do
+        let(:search_term) { 'defghi' }
 
-      it 'returns no runners' do
-        expect { subject }.to raise_error("Unexpected parent type: #{obj.class}")
+        it 'returns runners containing term in token' do
+          is_expected.to contain_exactly(offline_project_runner)
+        end
       end
     end
   end
diff --git a/spec/support/shared_examples/graphql/resolvers/runners_resolver_shared_examples.rb b/spec/support/shared_examples/graphql/resolvers/runners_resolver_shared_examples.rb
deleted file mode 100644
index d5f30e3aba94a0..00000000000000
--- a/spec/support/shared_examples/graphql/resolvers/runners_resolver_shared_examples.rb
+++ /dev/null
@@ -1,175 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.shared_examples_for Resolvers::Ci::RunnersResolver do
-  describe '#resolve' do
-    subject { resolve(described_class, ctx: { current_user: user }, args: args).items.to_a }
-
-    context 'with empty args' do
-      let(:args) do
-        {}
-      end
-
-      context 'when the user cannot see runners' do
-        let(:user) { build(:user) }
-
-        it 'returns no runners' do
-          is_expected.to be_empty
-        end
-      end
-
-      context 'without sort' do
-        it 'returns all the runners' do
-          is_expected.to contain_exactly(inactive_project_runner, offline_project_runner, group_runner, subgroup_runner, instance_runner)
-        end
-      end
-    end
-
-    context 'with a sort argument' do
-      context "set to :contacted_asc" do
-        let(:args) do
-          { sort: :contacted_asc }
-        end
-
-        it { is_expected.to eq([offline_project_runner, instance_runner, inactive_project_runner, group_runner, subgroup_runner]) }
-      end
-
-      context "set to :contacted_desc" do
-        let(:args) do
-          { sort: :contacted_desc }
-        end
-
-        it { is_expected.to eq([offline_project_runner, instance_runner, inactive_project_runner, group_runner, subgroup_runner].reverse) }
-      end
-
-      context "set to :created_at_desc" do
-        let(:args) do
-          { sort: :created_at_desc }
-        end
-
-        it { is_expected.to eq([instance_runner, subgroup_runner, group_runner, offline_project_runner, inactive_project_runner]) }
-      end
-
-      context "set to :created_at_asc" do
-        let(:args) do
-          { sort: :created_at_asc }
-        end
-
-        it { is_expected.to eq([instance_runner, subgroup_runner, group_runner, offline_project_runner, inactive_project_runner].reverse) }
-      end
-    end
-
-    context 'when type is filtered' do
-      let(:args) do
-        { type: runner_type.to_s }
-      end
-
-      context 'to instance runners' do
-        let(:runner_type) { :instance_type }
-
-        it 'returns the instance runner' do
-          is_expected.to contain_exactly(instance_runner)
-        end
-      end
-
-      context 'to group runners' do
-        let(:runner_type) { :group_type }
-
-        it 'returns the group runner' do
-          is_expected.to contain_exactly(group_runner, subgroup_runner)
-        end
-      end
-
-      context 'to project runners' do
-        let(:runner_type) { :project_type }
-
-        it 'returns the project runner' do
-          is_expected.to contain_exactly(inactive_project_runner, offline_project_runner)
-        end
-      end
-    end
-
-    context 'when status is filtered' do
-      let(:args) do
-        { status: runner_status.to_s }
-      end
-
-      context 'to active runners' do
-        let(:runner_status) { :active }
-
-        it 'returns the instance and group runners' do
-          is_expected.to contain_exactly(offline_project_runner, group_runner, subgroup_runner, instance_runner)
-        end
-      end
-
-      context 'to offline runners' do
-        let(:runner_status) { :offline }
-
-        it 'returns the offline project runner' do
-          is_expected.to contain_exactly(offline_project_runner)
-        end
-      end
-    end
-
-    context 'when tag list is filtered' do
-      let(:args) do
-        { tag_list: tag_list }
-      end
-
-      context 'with "project_runner" tag' do
-        let(:tag_list) { ['project_runner'] }
-
-        it 'returns the project_runner runners' do
-          is_expected.to contain_exactly(offline_project_runner, inactive_project_runner)
-        end
-      end
-
-      context 'with "project_runner" and "active_runner" tags as comma-separated string' do
-        let(:tag_list) { ['project_runner,active_runner'] }
-
-        it 'returns the offline_project_runner runner' do
-          is_expected.to contain_exactly(offline_project_runner)
-        end
-      end
-
-      context 'with "active_runner" and "instance_runner" tags as array' do
-        let(:tag_list) { %w[instance_runner active_runner] }
-
-        it 'returns the offline_project_runner runner' do
-          is_expected.to contain_exactly(instance_runner)
-        end
-      end
-    end
-
-    context 'when text is filtered' do
-      let(:args) do
-        { search: search_term }
-      end
-
-      context 'to "project"' do
-        let(:search_term) { 'project' }
-
-        it 'returns both project runners' do
-          is_expected.to contain_exactly(inactive_project_runner, offline_project_runner)
-        end
-      end
-
-      context 'to "group"' do
-        let(:search_term) { 'group' }
-
-        it 'returns group runners' do
-          is_expected.to contain_exactly(group_runner, subgroup_runner)
-        end
-      end
-
-      context 'to "defghi"' do
-        let(:search_term) { 'defghi' }
-
-        it 'returns runners containing term in token' do
-          is_expected.to contain_exactly(offline_project_runner)
-        end
-      end
-    end
-  end
-end
-- 
GitLab


From 39f6c863bc9d14a3ca82cec2e2b64bef1fa393bb Mon Sep 17 00:00:00 2001
From: Pedro Pombeiro <noreply@pedro.pombei.ro>
Date: Mon, 2 Aug 2021 18:19:27 +0200
Subject: [PATCH 09/10] Use RunnersFinder double in resolver tests

---
 .../ci/group_runners_resolver_spec.rb         | 288 ++++++------------
 1 file changed, 99 insertions(+), 189 deletions(-)

diff --git a/spec/graphql/resolvers/ci/group_runners_resolver_spec.rb b/spec/graphql/resolvers/ci/group_runners_resolver_spec.rb
index c5363d78e1d037..f0851d59828619 100644
--- a/spec/graphql/resolvers/ci/group_runners_resolver_spec.rb
+++ b/spec/graphql/resolvers/ci/group_runners_resolver_spec.rb
@@ -5,23 +5,38 @@
 RSpec.describe Resolvers::Ci::GroupRunnersResolver do
   include GraphqlHelpers
 
-  include_context 'runners resolver setup'
+  describe '#resolve' do
+    let(:args) { }
 
-  shared_context 'resolve args with membership' do
-    let(:membership) { nil }
-    let(:args) do
-      { membership: membership }.reject { |_, v| v.nil? }
+    subject { resolve(described_class, obj: obj, ctx: { current_user: user }, args: args) }
+
+    shared_context 'resolve args with membership' do
+      let(:membership) { nil }
+      let(:args) do
+        { membership: membership }.reject { |_, v| v.nil? }
+      end
     end
-  end
 
-  describe '#resolve' do
-    let(:args) { }
+    context 'with a RunnersFinder double' do
+      let_it_be(:user) { build(:user) }
 
-    subject { resolve(described_class, obj: obj, ctx: { current_user: user }, args: args).items.to_a }
+      let(:finder) { instance_double(::Ci::RunnersFinder) }
+      let(:expected_params) { :uninitialized }
 
-    context 'with user as group owner' do
       before do
-        group.add_owner(user)
+        allow(::Ci::RunnersFinder).to receive(:new).with(current_user: user, params: expected_params).once.and_return(finder)
+        allow(finder).to receive(:execute).once.and_return([:execute_return_value])
+      end
+
+      shared_examples 'a resolver delegating to RunnersFinder' do
+        it 'calls new RunnersFinder instance with expected parameters' do
+          is_expected.to be_an_instance_of(Gitlab::Graphql::Pagination::ArrayConnection)
+          expect(finder).to have_received(:execute).once
+        end
+
+        it 'returns the result from RunnersFinder.execute' do
+          expect(subject.items.to_a).to eq([:execute_return_value])
+        end
       end
 
       context 'with obj set to nil' do
@@ -40,250 +55,145 @@
         end
       end
 
-      context 'with obj set to subgroup' do
-        let(:obj) { subgroup }
+      context 'with obj set to group' do
+        let(:obj) { build(:group) }
 
         context 'with a membership argument' do
           include_context 'resolve args with membership'
 
           context 'set to :direct' do
             let(:membership) { :direct }
+            let(:expected_params) { a_hash_including(membership: membership) }
 
-            it { is_expected.to contain_exactly(subgroup_runner) }
+            it_behaves_like 'a resolver delegating to RunnersFinder'
           end
 
           context 'not set' do
-            it { is_expected.to contain_exactly(subgroup_runner) }
+            let(:membership) { }
+            let(:expected_params) { a_hash_including(membership: nil) }
+
+            it_behaves_like 'a resolver delegating to RunnersFinder'
           end
 
           context 'set to :descendants' do
             let(:membership) { :descendants }
+            let(:expected_params) { a_hash_including(membership: :descendants) }
 
-            it { is_expected.to contain_exactly(subgroup_runner) }
+            it_behaves_like 'a resolver delegating to RunnersFinder'
           end
         end
-      end
 
-      context 'with obj set to unsupported value' do
-        let_it_be(:obj) { build(:project) }
-
-        it 'raises error' do
-          expect { subject }.to raise_error('Expected group missing')
-        end
-      end
-
-      context 'with obj set to group' do
-        let(:obj) { group }
-
-        context 'with empty args' do
+        context 'when status is filtered' do
           let(:args) do
-            {}
+            { status: 'active' }
           end
 
-          context 'when the user cannot see runners' do
-            let(:user) { build(:user) }
+          let(:expected_params) { a_hash_including(status_status: 'active') }
 
-            it 'returns no runners' do
-              is_expected.to be_empty
-            end
-          end
-
-          context 'without sort' do
-            it 'returns all the runners' do
-              is_expected.to contain_exactly(inactive_project_runner, offline_project_runner, group_runner, subgroup_runner)
-            end
-          end
+          it_behaves_like 'a resolver delegating to RunnersFinder'
         end
 
-        context 'with a sort argument' do
-          context "set to :contacted_asc" do
-            let(:args) do
-              { sort: :contacted_asc }
-            end
-
-            it { is_expected.to eq([offline_project_runner, inactive_project_runner, group_runner, subgroup_runner]) }
-          end
-
-          context "set to :contacted_desc" do
-            let(:args) do
-              { sort: :contacted_desc }
-            end
-
-            it { is_expected.to eq([offline_project_runner, inactive_project_runner, group_runner, subgroup_runner].reverse) }
-          end
-
-          context "set to :created_at_desc" do
-            let(:args) do
-              { sort: :created_at_desc }
-            end
-
-            it { is_expected.to eq([subgroup_runner, group_runner, offline_project_runner, inactive_project_runner]) }
+        context 'when tag list is filtered' do
+          let(:args) do
+            { tag_list: %w(project_runner,active_runner) }
           end
 
-          context "set to :created_at_asc" do
-            let(:args) do
-              { sort: :created_at_asc }
-            end
+          let(:expected_params) { a_hash_including(tag_name: %w(project_runner,active_runner)) }
 
-            it { is_expected.to eq([subgroup_runner, group_runner, offline_project_runner, inactive_project_runner].reverse) }
-          end
+          it_behaves_like 'a resolver delegating to RunnersFinder'
         end
 
-        context 'when type is filtered' do
+        context 'when text is filtered' do
           let(:args) do
-            { type: runner_type.to_s }
+            { search: 'abc' }
           end
 
-          context 'to instance runners' do
-            let(:runner_type) { :instance_type }
-
-            it 'returns empty array' do
-              is_expected.to eq([])
-            end
-          end
+          let(:expected_params) { a_hash_including(search: 'abc') }
 
-          context 'to group runners' do
-            let(:runner_type) { :group_type }
-
-            it 'returns the group runner' do
-              is_expected.to contain_exactly(group_runner, subgroup_runner)
-            end
-          end
+          it_behaves_like 'a resolver delegating to RunnersFinder'
+        end
+      end
 
-          context 'to project runners' do
-            let(:runner_type) { :project_type }
+      context 'with obj set to unsupported value' do
+        let_it_be(:obj) { build(:project) }
 
-            it 'returns the project runner' do
-              is_expected.to contain_exactly(inactive_project_runner, offline_project_runner)
-            end
-          end
+        it 'raises error' do
+          expect { subject }.to raise_error('Expected group missing')
         end
+      end
+    end
 
-        context 'when status is filtered' do
-          let(:args) do
-            { status: runner_status.to_s }
-          end
+    context 'with a real RunnersFinder instance' do
+      include_context 'runners resolver setup'
 
-          context 'to active runners' do
-            let(:runner_status) { :active }
+      context 'with obj set to group' do
+        let(:obj) { group }
 
-            it 'returns the instance and group runners' do
-              is_expected.to contain_exactly(offline_project_runner, group_runner, subgroup_runner)
-            end
+        context 'with user as group owner' do
+          before do
+            group.add_owner(user)
           end
 
-          context 'to offline runners' do
-            let(:runner_status) { :offline }
-
-            it 'returns the offline project runner' do
-              is_expected.to contain_exactly(offline_project_runner)
+          context 'with empty args' do
+            let(:args) do
+              {}
             end
-          end
-        end
 
-        context 'when tag list is filtered' do
-          let(:args) do
-            { tag_list: tag_list }
-          end
-
-          context 'with "project_runner" tag' do
-            let(:tag_list) { ['project_runner'] }
+            context 'when the user cannot see runners' do
+              let(:user) { build(:user) }
 
-            it 'returns the project_runner runners' do
-              is_expected.to contain_exactly(offline_project_runner, inactive_project_runner)
+              it 'returns no runners' do
+                expect(subject.items.to_a).to be_empty
+              end
             end
-          end
-
-          context 'with "project_runner" and "active_runner" tags as comma-separated string' do
-            let(:tag_list) { ['project_runner,active_runner'] }
 
-            it 'returns the offline_project_runner runner' do
-              is_expected.to contain_exactly(offline_project_runner)
+            context 'without sort' do
+              it 'returns all the runners' do
+                expect(subject.items.to_a).to contain_exactly(inactive_project_runner, offline_project_runner, group_runner, subgroup_runner)
+              end
             end
           end
 
-          context 'with "active_runner" and "project_runner" tags as array' do
-            let(:tag_list) { %w[project_runner active_runner] }
+          context 'with a sort argument' do
+            context "set to :contacted_asc" do
+              let(:args) do
+                { sort: :contacted_asc }
+              end
 
-            it 'returns the offline_project_runner runner' do
-              is_expected.to contain_exactly(offline_project_runner)
+              it { expect(subject.items.to_a).to eq([offline_project_runner, inactive_project_runner, group_runner, subgroup_runner]) }
             end
           end
-        end
 
-        context 'when text is filtered' do
-          let(:args) do
-            { search: search_term }
-          end
-
-          context 'to "project"' do
-            let(:search_term) { 'project' }
-
-            it 'returns both project runners' do
-              is_expected.to contain_exactly(inactive_project_runner, offline_project_runner)
+          context 'when type is filtered' do
+            let(:args) do
+              { type: runner_type.to_s }
             end
-          end
 
-          context 'to "group"' do
-            let(:search_term) { 'group' }
+            context 'to instance runners' do
+              let(:runner_type) { :instance_type }
 
-            it 'returns group runners' do
-              is_expected.to contain_exactly(group_runner, subgroup_runner)
+              it 'returns empty array' do
+                expect(subject.items.to_a).to eq([])
+              end
             end
-          end
 
-          context 'to "defghi"' do
-            let(:search_term) { 'defghi' }
+            context 'to group runners' do
+              let(:runner_type) { :group_type }
 
-            it 'returns runners containing term in token' do
-              is_expected.to contain_exactly(offline_project_runner)
+              it 'returns the group runner' do
+                expect(subject.items.to_a).to contain_exactly(group_runner, subgroup_runner)
+              end
             end
           end
         end
 
-        context 'with a membership argument' do
-          include_context 'resolve args with membership'
-
-          shared_examples 'returns self and descendant runners' do
-            it { is_expected.to contain_exactly(group_runner, subgroup_runner, offline_project_runner, inactive_project_runner) }
-          end
-
-          context 'set to :direct' do
-            let(:membership) { :direct }
-
-            it { is_expected.to contain_exactly(group_runner) }
-          end
-
-          context 'not set' do
-            include_examples 'returns self and descendant runners'
-          end
-
-          context 'set to :descendants' do
-            let(:membership) { :descendants }
+        context 'when user has no access to the group' do
+          context 'with a membership argument' do
+            include_context 'resolve args with membership'
 
-            include_examples 'returns self and descendant runners'
-          end
-        end
-      end
-    end
-
-    context 'when user has no access to the group' do
-      context 'with obj set to subgroup' do
-        let(:obj) { subgroup }
-
-        context 'with a membership argument' do
-          include_context 'resolve args with membership'
-
-          context 'set to :direct' do
-            let(:membership) { :direct }
-
-            it { is_expected.to eq([]) }
-          end
-
-          context 'set to :descendants' do
             let(:membership) { :descendants }
 
-            it { is_expected.to eq([]) }
+            it { expect(subject.items.to_a).to eq([]) }
           end
         end
       end
-- 
GitLab


From 8b91fe0f8a3834d127a787e98b3aaccf3a52ab35 Mon Sep 17 00:00:00 2001
From: Pedro Pombeiro <noreply@pedro.pombei.ro>
Date: Thu, 5 Aug 2021 15:05:27 +0200
Subject: [PATCH 10/10] Apply MR review suggestion

---
 .../ci/group_runners_resolver_spec.rb         | 224 +++++-------------
 .../resolvers/ci/runners_resolver_spec.rb     | 206 ++++------------
 2 files changed, 104 insertions(+), 326 deletions(-)

diff --git a/spec/graphql/resolvers/ci/group_runners_resolver_spec.rb b/spec/graphql/resolvers/ci/group_runners_resolver_spec.rb
index f0851d59828619..89a2437a189f65 100644
--- a/spec/graphql/resolvers/ci/group_runners_resolver_spec.rb
+++ b/spec/graphql/resolvers/ci/group_runners_resolver_spec.rb
@@ -6,196 +6,88 @@
   include GraphqlHelpers
 
   describe '#resolve' do
-    let(:args) { }
-
     subject { resolve(described_class, obj: obj, ctx: { current_user: user }, args: args) }
 
-    shared_context 'resolve args with membership' do
-      let(:membership) { nil }
-      let(:args) do
-        { membership: membership }.reject { |_, v| v.nil? }
-      end
-    end
+    include_context 'runners resolver setup'
 
-    context 'with a RunnersFinder double' do
-      let_it_be(:user) { build(:user) }
+    let(:obj) { group }
+    let(:args) { {} }
 
-      let(:finder) { instance_double(::Ci::RunnersFinder) }
-      let(:expected_params) { :uninitialized }
+    # First, we can do a couple of basic real tests to verify common cases. That ensures that the code works.
+    context 'when user cannot see runners' do
+      it 'returns no runners' do
+        expect(subject.items.to_a).to eq([])
+      end
+    end
 
+    context 'with user as group owner' do
       before do
-        allow(::Ci::RunnersFinder).to receive(:new).with(current_user: user, params: expected_params).once.and_return(finder)
-        allow(finder).to receive(:execute).once.and_return([:execute_return_value])
+        group.add_owner(user)
       end
 
-      shared_examples 'a resolver delegating to RunnersFinder' do
-        it 'calls new RunnersFinder instance with expected parameters' do
-          is_expected.to be_an_instance_of(Gitlab::Graphql::Pagination::ArrayConnection)
-          expect(finder).to have_received(:execute).once
-        end
-
-        it 'returns the result from RunnersFinder.execute' do
-          expect(subject.items.to_a).to eq([:execute_return_value])
-        end
+      it 'returns all the runners' do
+        expect(subject.items.to_a).to contain_exactly(inactive_project_runner, offline_project_runner, group_runner, subgroup_runner)
       end
 
-      context 'with obj set to nil' do
-        let(:obj) { nil }
-
-        context 'with a membership argument' do
-          include_context 'resolve args with membership'
-
-          context 'set to :direct' do
-            let(:membership) { :direct }
+      context 'with membership direct' do
+        let(:args) { { membership: :direct } }
 
-            it 'raises an error' do
-              expect { subject }.to raise_error('Expected group missing')
-            end
-          end
+        it 'returns only direct runners' do
+          expect(subject.items.to_a).to contain_exactly(group_runner)
         end
       end
+    end
 
-      context 'with obj set to group' do
-        let(:obj) { build(:group) }
-
-        context 'with a membership argument' do
-          include_context 'resolve args with membership'
-
-          context 'set to :direct' do
-            let(:membership) { :direct }
-            let(:expected_params) { a_hash_including(membership: membership) }
-
-            it_behaves_like 'a resolver delegating to RunnersFinder'
-          end
-
-          context 'not set' do
-            let(:membership) { }
-            let(:expected_params) { a_hash_including(membership: nil) }
-
-            it_behaves_like 'a resolver delegating to RunnersFinder'
-          end
-
-          context 'set to :descendants' do
-            let(:membership) { :descendants }
-            let(:expected_params) { a_hash_including(membership: :descendants) }
-
-            it_behaves_like 'a resolver delegating to RunnersFinder'
-          end
-        end
-
-        context 'when status is filtered' do
-          let(:args) do
-            { status: 'active' }
-          end
-
-          let(:expected_params) { a_hash_including(status_status: 'active') }
-
-          it_behaves_like 'a resolver delegating to RunnersFinder'
-        end
-
-        context 'when tag list is filtered' do
-          let(:args) do
-            { tag_list: %w(project_runner,active_runner) }
-          end
-
-          let(:expected_params) { a_hash_including(tag_name: %w(project_runner,active_runner)) }
-
-          it_behaves_like 'a resolver delegating to RunnersFinder'
-        end
-
-        context 'when text is filtered' do
-          let(:args) do
-            { search: 'abc' }
-          end
-
-          let(:expected_params) { a_hash_including(search: 'abc') }
+    # Then, we can check specific edge cases for this resolver
+    context 'with obj set to nil' do
+      let(:obj) { nil }
 
-          it_behaves_like 'a resolver delegating to RunnersFinder'
-        end
+      it 'raises an error' do
+        expect { subject }.to raise_error('Expected group missing')
       end
+    end
 
-      context 'with obj set to unsupported value' do
-        let_it_be(:obj) { build(:project) }
+    context 'with obj not set to group' do
+      let(:obj) { build(:project) }
 
-        it 'raises error' do
-          expect { subject }.to raise_error('Expected group missing')
-        end
+      it 'raises an error' do
+        expect { subject }.to raise_error('Expected group missing')
       end
     end
 
-    context 'with a real RunnersFinder instance' do
-      include_context 'runners resolver setup'
-
-      context 'with obj set to group' do
-        let(:obj) { group }
-
-        context 'with user as group owner' do
-          before do
-            group.add_owner(user)
-          end
-
-          context 'with empty args' do
-            let(:args) do
-              {}
-            end
-
-            context 'when the user cannot see runners' do
-              let(:user) { build(:user) }
-
-              it 'returns no runners' do
-                expect(subject.items.to_a).to be_empty
-              end
-            end
-
-            context 'without sort' do
-              it 'returns all the runners' do
-                expect(subject.items.to_a).to contain_exactly(inactive_project_runner, offline_project_runner, group_runner, subgroup_runner)
-              end
-            end
-          end
-
-          context 'with a sort argument' do
-            context "set to :contacted_asc" do
-              let(:args) do
-                { sort: :contacted_asc }
-              end
-
-              it { expect(subject.items.to_a).to eq([offline_project_runner, inactive_project_runner, group_runner, subgroup_runner]) }
-            end
-          end
-
-          context 'when type is filtered' do
-            let(:args) do
-              { type: runner_type.to_s }
-            end
-
-            context 'to instance runners' do
-              let(:runner_type) { :instance_type }
-
-              it 'returns empty array' do
-                expect(subject.items.to_a).to eq([])
-              end
-            end
-
-            context 'to group runners' do
-              let(:runner_type) { :group_type }
-
-              it 'returns the group runner' do
-                expect(subject.items.to_a).to contain_exactly(group_runner, subgroup_runner)
-              end
-            end
-          end
-        end
+    # Here we have a mocked part. We assume that all possible edge cases are covered in RunnersFinder spec. So we don't need to test them twice.
+    # Only thing we can do is to verify that args from the resolver is correctly transformed to params of the Finder and we return the Finder's result back.
+    describe 'Allowed query arguments' do
+      let(:finder) { instance_double(::Ci::RunnersFinder) }
+      let(:args) do
+        {
+          status: 'active',
+          type: :group_type,
+          tag_list: ['active_runner'],
+          search: 'abc',
+          sort: :contacted_asc,
+          membership: :descendants
+        }
+      end
 
-        context 'when user has no access to the group' do
-          context 'with a membership argument' do
-            include_context 'resolve args with membership'
+      let(:expected_params) do
+        {
+          status_status: 'active',
+          type_type: :group_type,
+          tag_name: ['active_runner'],
+          preload: { tag_name: nil },
+          search: 'abc',
+          sort: 'contacted_asc',
+          membership: :descendants,
+          group: group
+        }
+      end
 
-            let(:membership) { :descendants }
+      it 'calls RunnersFinder with expected arguments' do
+        allow(::Ci::RunnersFinder).to receive(:new).with(current_user: user, params: expected_params).once.and_return(finder)
+        allow(finder).to receive(:execute).once.and_return([:execute_return_value])
 
-            it { expect(subject.items.to_a).to eq([]) }
-          end
-        end
+        expect(subject.items.to_a).to eq([:execute_return_value])
       end
     end
   end
diff --git a/spec/graphql/resolvers/ci/runners_resolver_spec.rb b/spec/graphql/resolvers/ci/runners_resolver_spec.rb
index bbaf08c9628a0f..bb8dadeca40cf8 100644
--- a/spec/graphql/resolvers/ci/runners_resolver_spec.rb
+++ b/spec/graphql/resolvers/ci/runners_resolver_spec.rb
@@ -5,184 +5,70 @@
 RSpec.describe Resolvers::Ci::RunnersResolver do
   include GraphqlHelpers
 
-  include_context 'runners resolver setup'
-
   describe '#resolve' do
     let(:obj) { nil }
+    let(:args) { {} }
 
-    subject { resolve(described_class, obj: obj, ctx: { current_user: user }, args: args).items.to_a }
-
-    context 'with empty args' do
-      let(:args) do
-        {}
-      end
-
-      context 'with obj set to unsupported value' do
-        let_it_be(:obj) { build(:group) }
-
-        it 'raises error' do
-          expect { subject }.to raise_error("Unexpected parent type: #{obj.class}")
-        end
-      end
-
-      context 'when the user cannot see runners' do
-        let(:user) { build(:user) }
-
-        it 'returns no runners' do
-          is_expected.to be_empty
-        end
-      end
-
-      context 'without sort' do
-        it 'returns all the runners' do
-          is_expected.to contain_exactly(inactive_project_runner, offline_project_runner, group_runner, subgroup_runner, instance_runner)
-        end
-      end
-    end
-
-    context 'with a sort argument' do
-      context "set to :contacted_asc" do
-        let(:args) do
-          { sort: :contacted_asc }
-        end
-
-        it { is_expected.to eq([offline_project_runner, instance_runner, inactive_project_runner, group_runner, subgroup_runner]) }
-      end
-
-      context "set to :contacted_desc" do
-        let(:args) do
-          { sort: :contacted_desc }
-        end
-
-        it { is_expected.to eq([offline_project_runner, instance_runner, inactive_project_runner, group_runner, subgroup_runner].reverse) }
-      end
-
-      context "set to :created_at_desc" do
-        let(:args) do
-          { sort: :created_at_desc }
-        end
-
-        it { is_expected.to eq([instance_runner, subgroup_runner, group_runner, offline_project_runner, inactive_project_runner]) }
-      end
-
-      context "set to :created_at_asc" do
-        let(:args) do
-          { sort: :created_at_asc }
-        end
-
-        it { is_expected.to eq([instance_runner, subgroup_runner, group_runner, offline_project_runner, inactive_project_runner].reverse) }
-      end
-    end
-
-    context 'when type is filtered' do
-      let(:args) do
-        { type: runner_type.to_s }
-      end
-
-      context 'to instance runners' do
-        let(:runner_type) { :instance_type }
-
-        it 'returns the instance runner' do
-          is_expected.to contain_exactly(instance_runner)
-        end
-      end
-
-      context 'to group runners' do
-        let(:runner_type) { :group_type }
+    subject { resolve(described_class, obj: obj, ctx: { current_user: user }, args: args) }
 
-        it 'returns the group runner' do
-          is_expected.to contain_exactly(group_runner, subgroup_runner)
-        end
-      end
+    include_context 'runners resolver setup'
 
-      context 'to project runners' do
-        let(:runner_type) { :project_type }
+    # First, we can do a couple of basic real tests to verify common cases. That ensures that the code works.
+    context 'when user cannot see runners' do
+      let(:user) { build(:user) }
 
-        it 'returns the project runner' do
-          is_expected.to contain_exactly(inactive_project_runner, offline_project_runner)
-        end
+      it 'returns no runners' do
+        expect(subject.items.to_a).to eq([])
       end
     end
 
-    context 'when status is filtered' do
-      let(:args) do
-        { status: runner_status.to_s }
-      end
-
-      context 'to active runners' do
-        let(:runner_status) { :active }
+    context 'when user can see runners' do
+      let(:obj) { nil }
 
-        it 'returns the instance and group runners' do
-          is_expected.to contain_exactly(offline_project_runner, group_runner, subgroup_runner, instance_runner)
-        end
-      end
-
-      context 'to offline runners' do
-        let(:runner_status) { :offline }
-
-        it 'returns the offline project runner' do
-          is_expected.to contain_exactly(offline_project_runner)
-        end
+      it 'returns all the runners' do
+        expect(subject.items.to_a).to contain_exactly(inactive_project_runner, offline_project_runner, group_runner, subgroup_runner, instance_runner)
       end
     end
 
-    context 'when tag list is filtered' do
-      let(:args) do
-        { tag_list: tag_list }
-      end
-
-      context 'with "project_runner" tag' do
-        let(:tag_list) { ['project_runner'] }
+    # Then, we can check specific edge cases for this resolver
+    context 'with obj not set to nil' do
+      let(:obj) { build(:project) }
 
-        it 'returns the project_runner runners' do
-          is_expected.to contain_exactly(offline_project_runner, inactive_project_runner)
-        end
-      end
-
-      context 'with "project_runner" and "active_runner" tags as comma-separated string' do
-        let(:tag_list) { ['project_runner,active_runner'] }
-
-        it 'returns the offline_project_runner runner' do
-          is_expected.to contain_exactly(offline_project_runner)
-        end
-      end
-
-      context 'with "active_runner" and "instance_runner" tags as array' do
-        let(:tag_list) { %w[instance_runner active_runner] }
-
-        it 'returns the instance_runner runner' do
-          is_expected.to contain_exactly(instance_runner)
-        end
+      it 'raises an error' do
+        expect { subject }.to raise_error(a_string_including('Unexpected parent type'))
       end
     end
 
-    context 'when text is filtered' do
+    # Here we have a mocked part. We assume that all possible edge cases are covered in RunnersFinder spec. So we don't need to test them twice.
+    # Only thing we can do is to verify that args from the resolver is correctly transformed to params of the Finder and we return the Finder's result back.
+    describe 'Allowed query arguments' do
+      let(:finder) { instance_double(::Ci::RunnersFinder) }
       let(:args) do
-        { search: search_term }
-      end
-
-      context 'to "project"' do
-        let(:search_term) { 'project' }
-
-        it 'returns both project runners' do
-          is_expected.to contain_exactly(inactive_project_runner, offline_project_runner)
-        end
-      end
-
-      context 'to "group"' do
-        let(:search_term) { 'group' }
-
-        it 'returns group runners' do
-          is_expected.to contain_exactly(group_runner, subgroup_runner)
-        end
-      end
-
-      context 'to "defghi"' do
-        let(:search_term) { 'defghi' }
-
-        it 'returns runners containing term in token' do
-          is_expected.to contain_exactly(offline_project_runner)
-        end
+        {
+          status: 'active',
+          type: :instance_type,
+          tag_list: ['active_runner'],
+          search: 'abc',
+          sort: :contacted_asc
+        }
+      end
+
+      let(:expected_params) do
+        {
+          status_status: 'active',
+          type_type: :instance_type,
+          tag_name: ['active_runner'],
+          preload: { tag_name: nil },
+          search: 'abc',
+          sort: 'contacted_asc'
+        }
+      end
+
+      it 'calls RunnersFinder with expected arguments' do
+        allow(::Ci::RunnersFinder).to receive(:new).with(current_user: user, params: expected_params).once.and_return(finder)
+        allow(finder).to receive(:execute).once.and_return([:execute_return_value])
+
+        expect(subject.items.to_a).to eq([:execute_return_value])
       end
     end
   end
-- 
GitLab