From 74399a90989e5caade1de1833a7f65cfbc070bcd Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Fri, 24 May 2019 02:07:59 +0800 Subject: Show Test Automation Engineer for spec/feature as reviewers. --- danger/roulette/Dangerfile | 10 +++-- lib/gitlab/danger/helper.rb | 4 +- lib/gitlab/danger/teammate.rb | 27 +++++++++---- spec/features/admin/admin_appearance_spec.rb | 2 + spec/features/atom/dashboard_issues_spec.rb | 2 + spec/features/groups/merge_requests_spec.rb | 2 + .../conversational_development_index_spec.rb | 2 + spec/features/issues_spec.rb | 2 + .../user_merges_merge_request_spec.rb | 2 + spec/features/project_variables_spec.rb | 2 + spec/features/projects/clusters_spec.rb | 2 + spec/features/projects/pipelines/pipeline_spec.rb | 2 + spec/features/projects_spec.rb | 2 + spec/features/security/profile_access_spec.rb | 2 + spec/lib/gitlab/danger/teammate_spec.rb | 47 +++++++++++++++++----- 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 -- cgit v1.2.1