summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSean McGivern <sean@gitlab.com>2019-06-06 14:54:21 +0000
committerSean McGivern <sean@gitlab.com>2019-06-06 14:54:21 +0000
commit86e2f1339bdfe477e6d1d630cded24c21d778cdb (patch)
tree2ceb4f416eb6813ff784e0f43c8c95209329a4e6
parent345600682653aa2a5b1e15a104b36291b39d5599 (diff)
parent74399a90989e5caade1de1833a7f65cfbc070bcd (diff)
downloadgitlab-ce-86e2f1339bdfe477e6d1d630cded24c21d778cdb.tar.gz
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
-rw-r--r--danger/roulette/Dangerfile10
-rw-r--r--lib/gitlab/danger/helper.rb4
-rw-r--r--lib/gitlab/danger/teammate.rb27
-rw-r--r--spec/features/admin/admin_appearance_spec.rb2
-rw-r--r--spec/features/atom/dashboard_issues_spec.rb2
-rw-r--r--spec/features/groups/merge_requests_spec.rb2
-rw-r--r--spec/features/instance_statistics/conversational_development_index_spec.rb2
-rw-r--r--spec/features/issues_spec.rb2
-rw-r--r--spec/features/merge_request/user_merges_merge_request_spec.rb2
-rw-r--r--spec/features/project_variables_spec.rb2
-rw-r--r--spec/features/projects/clusters_spec.rb2
-rw-r--r--spec/features/projects/pipelines/pipeline_spec.rb2
-rw-r--r--spec/features/projects_spec.rb2
-rw-r--r--spec/features/security/profile_access_spec.rb2
-rw-r--r--spec/lib/gitlab/danger/teammate_spec.rb47
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