Merge branch '62015-add-counterpart-tae-to-the-reviewer-roulette' into 'master'

Resolve "Add counterpart TAE to the reviewer roulette"

Closes #62015

See merge request gitlab-org/gitlab-ce!28678

15 files changed, 89 insertions, 21 deletions

diff --git a/danger/roulette/Dangerfile b/danger/roulette/Dangerfile
index de314c5b39f..a0f1447e76a 100644
--- a/danger/roulette/Dangerfile
+++ b/danger/roulette/Dangerfile
@@ -33,10 +33,14 @@ MARKDOWN
 
 def spin_for_category(team, project, category, branch_name)
   random = roulette.new_random(branch_name)
+  labels = gitlab.mr_labels
 
-  reviewers = team.select { |member| member.reviewer?(project, category) }
-  traintainers = team.select { |member| member.traintainer?(project, category) }
-  maintainers = team.select { |member| member.maintainer?(project, category) }
+  reviewers, traintainers, maintainers =
+    %i[reviewer? traintainer? maintainer?].map do |kind|
+      team.select do |member|
+        member.public_send(kind, project, category, labels) # rubocop:disable GitlabSecurity/PublicSend
+      end
+    end
 
   # TODO: take CODEOWNERS into account?
   # https://gitlab.com/gitlab-org/gitlab-ce/issues/57653
diff --git a/lib/gitlab/danger/helper.rb b/lib/gitlab/danger/helper.rb
index 7effb802678..7a0fb419f8e 100644
--- a/lib/gitlab/danger/helper.rb
+++ b/lib/gitlab/danger/helper.rb
@@ -72,7 +72,8 @@ module Gitlab
       CATEGORY_LABELS = {
         docs: "~Documentation", # Docs are reviewed along DevOps stages, so don't need roulette for now.
         none: "",
-        qa: "~QA"
+        qa: "~QA",
+        test: "~test for `spec/features/*`"
       }.freeze
       CATEGORIES = {
         %r{\Adoc/} => :none, # To reinstate roulette for documentation, set to `:docs`.
@@ -104,6 +105,7 @@ module Gitlab
 
         %r{\A(ee/)?app/(?!assets|views)[^/]+} => :backend,
         %r{\A(ee/)?(bin|config|danger|generator_templates|lib|rubocop|scripts)/} => :backend,
+        %r{\A(ee/)?spec/features/} => :test,
         %r{\A(ee/)?spec/(?!javascripts|frontend)[^/]+} => :backend,
         %r{\A(ee/)?vendor/(?!assets)[^/]+} => :backend,
         %r{\A(ee/)?vendor/(languages\.yml|licenses\.csv)\z} => :backend,
diff --git a/lib/gitlab/danger/teammate.rb b/lib/gitlab/danger/teammate.rb
index c4e66da8ed1..b44f134f2c1 100644
--- a/lib/gitlab/danger/teammate.rb
+++ b/lib/gitlab/danger/teammate.rb
@@ -3,11 +3,12 @@
 module Gitlab
   module Danger
     class Teammate
-      attr_reader :name, :username, :projects
+      attr_reader :name, :username, :role, :projects
 
       def initialize(options = {})
         @username = options['username']
         @name = options['name'] || @username
+        @role = options['role']
         @projects = options['projects']
       end
 
@@ -20,20 +21,32 @@ module Gitlab
       end
 
       # Traintainers also count as reviewers
-      def reviewer?(project, category)
-        capabilities(project).include?("reviewer #{category}") || traintainer?(project, category)
+      def reviewer?(project, category, labels)
+        has_capability?(project, category, :reviewer, labels) ||
+          traintainer?(project, category, labels)
       end
 
-      def traintainer?(project, category)
-        capabilities(project).include?("trainee_maintainer #{category}")
+      def traintainer?(project, category, labels)
+        has_capability?(project, category, :trainee_maintainer, labels)
       end
 
-      def maintainer?(project, category)
-        capabilities(project).include?("maintainer #{category}")
+      def maintainer?(project, category, labels)
+        has_capability?(project, category, :maintainer, labels)
       end
 
       private
 
+      def has_capability?(project, category, kind, labels)
+        case category
+        when :test
+          area = role[/Test Automation Engineer, (\w+)/, 1]
+
+          area && labels.any?(area) if kind == :reviewer
+        else
+          capabilities(project).include?("#{kind} #{category}")
+        end
+      end
+
       def capabilities(project)
         Array(projects.fetch(project, []))
       end
diff --git a/spec/features/admin/admin_appearance_spec.rb b/spec/features/admin/admin_appearance_spec.rb
index 83cd686818c..f6c498f7a4c 100644
--- a/spec/features/admin/admin_appearance_spec.rb
+++ b/spec/features/admin/admin_appearance_spec.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'spec_helper'
 
 describe 'Admin Appearance' do
diff --git a/spec/features/atom/dashboard_issues_spec.rb b/spec/features/atom/dashboard_issues_spec.rb
index dfa1c92ea49..d523e2992db 100644
--- a/spec/features/atom/dashboard_issues_spec.rb
+++ b/spec/features/atom/dashboard_issues_spec.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'spec_helper'
 
 describe "Dashboard Issues Feed" do
diff --git a/spec/features/groups/merge_requests_spec.rb b/spec/features/groups/merge_requests_spec.rb
index e1bc4eca619..59230d6891a 100644
--- a/spec/features/groups/merge_requests_spec.rb
+++ b/spec/features/groups/merge_requests_spec.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'spec_helper'
 
 describe 'Group merge requests page' do
diff --git a/spec/features/instance_statistics/conversational_development_index_spec.rb b/spec/features/instance_statistics/conversational_development_index_spec.rb
index d8be554d734..713cd944f8c 100644
--- a/spec/features/instance_statistics/conversational_development_index_spec.rb
+++ b/spec/features/instance_statistics/conversational_development_index_spec.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'spec_helper'
 
 describe 'Conversational Development Index' do
diff --git a/spec/features/issues_spec.rb b/spec/features/issues_spec.rb
index bc0ec58bd24..5ee9425c491 100644
--- a/spec/features/issues_spec.rb
+++ b/spec/features/issues_spec.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'spec_helper'
 
 describe 'Issues' do
diff --git a/spec/features/merge_request/user_merges_merge_request_spec.rb b/spec/features/merge_request/user_merges_merge_request_spec.rb
index 6539e6e9208..da15a4bda4b 100644
--- a/spec/features/merge_request/user_merges_merge_request_spec.rb
+++ b/spec/features/merge_request/user_merges_merge_request_spec.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require "spec_helper"
 
 describe "User merges a merge request", :js do
diff --git a/spec/features/project_variables_spec.rb b/spec/features/project_variables_spec.rb
index 76abc640077..95685a3c7ff 100644
--- a/spec/features/project_variables_spec.rb
+++ b/spec/features/project_variables_spec.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'spec_helper'
 
 describe 'Project variables', :js do
diff --git a/spec/features/projects/clusters_spec.rb b/spec/features/projects/clusters_spec.rb
index a85e7333ba8..ce382c19fc1 100644
--- a/spec/features/projects/clusters_spec.rb
+++ b/spec/features/projects/clusters_spec.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'spec_helper'
 
 describe 'Clusters', :js do
diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb
index 25b3ac00604..1de153db41c 100644
--- a/spec/features/projects/pipelines/pipeline_spec.rb
+++ b/spec/features/projects/pipelines/pipeline_spec.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'spec_helper'
 
 describe 'Pipeline', :js do
diff --git a/spec/features/projects_spec.rb b/spec/features/projects_spec.rb
index 27f6ed56283..b5112758475 100644
--- a/spec/features/projects_spec.rb
+++ b/spec/features/projects_spec.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'spec_helper'
 
 describe 'Project' do
diff --git a/spec/features/security/profile_access_spec.rb b/spec/features/security/profile_access_spec.rb
index a198e65046f..044a47567be 100644
--- a/spec/features/security/profile_access_spec.rb
+++ b/spec/features/security/profile_access_spec.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'spec_helper'
 
 describe "Profile access" do
diff --git a/spec/lib/gitlab/danger/teammate_spec.rb b/spec/lib/gitlab/danger/teammate_spec.rb
index 753c74ff814..6a6cf1429c8 100644
--- a/spec/lib/gitlab/danger/teammate_spec.rb
+++ b/spec/lib/gitlab/danger/teammate_spec.rb
@@ -5,39 +5,66 @@ require 'fast_spec_helper'
 require 'gitlab/danger/teammate'
 
 describe Gitlab::Danger::Teammate do
-  subject { described_class.new({ 'projects' => projects }) }
+  subject { described_class.new(options) }
+  let(:options) { { 'projects' => projects, 'role' => role } }
   let(:projects) { { project => capabilities } }
+  let(:role) { 'Engineer, Manage' }
+  let(:labels) { [] }
   let(:project) { double }
 
-  describe 'multiple roles project project' do
-    let(:capabilities) { ['reviewer backend', 'maintainer frontend', 'trainee_maintainer database'] }
+  context 'when having multiple capabilities' do
+    let(:capabilities) { ['reviewer backend', 'maintainer frontend', 'trainee_maintainer qa'] }
 
     it '#reviewer? supports multiple roles per project' do
-      expect(subject.reviewer?(project, :backend)).to be_truthy
+      expect(subject.reviewer?(project, :backend, labels)).to be_truthy
     end
 
     it '#traintainer? supports multiple roles per project' do
-      expect(subject.traintainer?(project, :database)).to be_truthy
+      expect(subject.traintainer?(project, :qa, labels)).to be_truthy
     end
 
     it '#maintainer? supports multiple roles per project' do
-      expect(subject.maintainer?(project, :frontend)).to be_truthy
+      expect(subject.maintainer?(project, :frontend, labels)).to be_truthy
+    end
+
+    context 'when labels contain Create and the category is test' do
+      let(:labels) { ['Create'] }
+
+      context 'when role is Test Automation Engineer, Create' do
+        let(:role) { 'Test Automation Engineer, Create' }
+
+        it '#reviewer? returns true' do
+          expect(subject.reviewer?(project, :test, labels)).to be_truthy
+        end
+
+        it '#maintainer? returns false' do
+          expect(subject.maintainer?(project, :test, labels)).to be_falsey
+        end
+      end
+
+      context 'when role is Test Automation Engineer, Manage' do
+        let(:role) { 'Test Automation Engineer, Manage' }
+
+        it '#reviewer? returns false' do
+          expect(subject.reviewer?(project, :test, labels)).to be_falsey
+        end
+      end
     end
   end
 
-  describe 'one role project project' do
+  context 'when having single capability' do
     let(:capabilities) { 'reviewer backend' }
 
     it '#reviewer? supports one role per project' do
-      expect(subject.reviewer?(project, :backend)).to be_truthy
+      expect(subject.reviewer?(project, :backend, labels)).to be_truthy
     end
 
     it '#traintainer? supports one role per project' do
-      expect(subject.traintainer?(project, :database)).to be_falsey
+      expect(subject.traintainer?(project, :database, labels)).to be_falsey
     end
 
     it '#maintainer? supports one role per project' do
-      expect(subject.maintainer?(project, :frontend)).to be_falsey
+      expect(subject.maintainer?(project, :frontend, labels)).to be_falsey
     end
   end
 end