Commit 80da8dc4 authored by Rémy Coutable's avatar Rémy Coutable

Make sure SET maintainer is required when MR author is an SET

That will improve the reviewer roulette behavior for SETs.
Signed-off-by: default avatarRémy Coutable <remy@rymai.me>
parent 322336c3
......@@ -53,7 +53,7 @@ RSpec.describe Tooling::Danger::Roulette do
'username' => 'software-engineer-in-test',
'name' => 'Software Engineer in Test',
'role' => 'Software Engineer in Test, Create:Source Code',
'projects' => { 'gitlab' => 'reviewer qa', 'gitlab-qa' => 'maintainer' },
'projects' => { 'gitlab' => 'maintainer qa', 'gitlab-qa' => 'maintainer' },
'available' => true,
'tz_offset_hours' => 2.0
)
......@@ -176,8 +176,24 @@ RSpec.describe Tooling::Danger::Roulette do
context 'when change contains QA category' do
let(:categories) { [:qa] }
it 'assigns QA maintainer' do
expect(spins).to eq([described_class::Spin.new(:qa, nil, software_engineer_in_test, false, false)])
end
end
context 'when change contains QA category and another category' do
let(:categories) { [:backend, :qa] }
it 'assigns QA maintainer' do
expect(spins).to eq([described_class::Spin.new(:backend, engineering_productivity_reviewer, backend_maintainer, false, false), described_class::Spin.new(:qa, nil, software_engineer_in_test, :maintainer, false)])
end
context 'and author is an SET' do
let!(:author) { Tooling::Danger::Teammate.new('username' => software_engineer_in_test.username) }
it 'assigns QA reviewer' do
expect(spins).to eq([described_class::Spin.new(:qa, software_engineer_in_test, nil, false, false)])
expect(spins).to eq([described_class::Spin.new(:backend, engineering_productivity_reviewer, backend_maintainer, false, false), described_class::Spin.new(:qa, nil, nil, false, false)])
end
end
end
......
......@@ -2,6 +2,7 @@
require_relative '../../../tooling/danger/teammate'
require 'active_support/testing/time_helpers'
require 'rspec-parameterized'
RSpec.describe Tooling::Danger::Teammate do
using RSpec::Parameterized::TableSyntax
......@@ -48,6 +49,13 @@ RSpec.describe Tooling::Danger::Teammate do
context 'when having multiple capabilities' do
let(:capabilities) { ['reviewer backend', 'maintainer frontend', 'trainee_maintainer qa'] }
it '#any_capability? returns true if the person has any capability for the category in the given project' do
expect(subject.any_capability?(project, :backend)).to be_truthy
expect(subject.any_capability?(project, :frontend)).to be_truthy
expect(subject.any_capability?(project, :qa)).to be_truthy
expect(subject.any_capability?(project, :engineering_productivity)).to be_falsey
end
it '#reviewer? supports multiple roles per project' do
expect(subject.reviewer?(project, :backend, labels)).to be_truthy
end
......
......@@ -39,7 +39,7 @@ module Tooling
case spin.category
when :qa
# MR includes QA changes, but also other changes, and author isn't an SET
if categories.size > 1 && !team_mr_author&.reviewer?(project, spin.category, [])
if categories.size > 1 && !team_mr_author&.any_capability?(project, spin.category)
spin.optional_role = :maintainer
end
when :test
......
......@@ -33,6 +33,10 @@ module Tooling
projects&.has_key?(name)
end
def any_capability?(project, category)
capabilities(project).any? { |capability| capability.end_with?(category.to_s) }
end
def reviewer?(project, category, labels)
has_capability?(project, category, :reviewer, labels)
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment