Commit edaf718f authored by Doug Stull's avatar Doug Stull Committed by Albert Salim

Introduce reduced capacity to danger roulette

- sometimes we all need a break.
parent 5b43c227
......@@ -2,6 +2,8 @@
require_relative 'teammate'
require_relative 'request_helper' unless defined?(Gitlab::Danger::RequestHelper)
require_relative 'weightage/reviewers'
require_relative 'weightage/maintainers'
module Gitlab
module Danger
......@@ -151,20 +153,14 @@ module Gitlab
%i[reviewer traintainer maintainer].map do |role|
spin_role_for_category(team, role, project, category)
end
hungry_reviewers = reviewers.select { |member| member.hungry }
hungry_traintainers = traintainers.select { |member| member.hungry }
# TODO: take CODEOWNERS into account?
# https://gitlab.com/gitlab-org/gitlab/issues/26723
random = new_random(mr_source_branch)
# Make hungry traintainers have 4x the chance to be picked as a reviewer
# Make traintainers have 3x the chance to be picked as a reviewer
# Make hungry reviewers have 2x the chance to be picked as a reviewer
weighted_reviewers = reviewers + hungry_reviewers + traintainers + traintainers + traintainers + hungry_traintainers
weighted_reviewers = Weightage::Reviewers.new(reviewers, traintainers).execute
weighted_maintainers = Weightage::Maintainers.new(maintainers).execute
reviewer = spin_for_person(weighted_reviewers, random: random, timezone_experiment: timezone_experiment)
maintainer = spin_for_person(maintainers, random: random, timezone_experiment: timezone_experiment)
maintainer = spin_for_person(weighted_maintainers, random: random, timezone_experiment: timezone_experiment)
Spin.new(category, reviewer, maintainer, false, timezone_experiment)
end
......
......@@ -3,7 +3,7 @@
module Gitlab
module Danger
class Teammate
attr_reader :options, :username, :name, :role, :projects, :available, :hungry, :tz_offset_hours
attr_reader :options, :username, :name, :role, :projects, :available, :hungry, :reduced_capacity, :tz_offset_hours
# The options data are produced by https://gitlab.com/gitlab-org/gitlab-roulette/-/blob/master/lib/team_member.rb
def initialize(options = {})
......@@ -15,6 +15,7 @@ module Gitlab
@projects = options['projects']
@available = options['available']
@hungry = options['hungry']
@reduced_capacity = options['reduced_capacity']
@tz_offset_hours = options['tz_offset_hours']
end
......
# frozen_string_literal: true
module Gitlab
module Danger
module Weightage
CAPACITY_MULTIPLIER = 2 # change this number to change what it means to be a reduced capacity reviewer 1/this number
BASE_REVIEWER_WEIGHT = 1
end
end
end
# frozen_string_literal: true
require_relative '../weightage'
module Gitlab
module Danger
module Weightage
class Maintainers
def initialize(maintainers)
@maintainers = maintainers
end
def execute
maintainers.each_with_object([]) do |maintainer, weighted_maintainers|
add_weighted_reviewer(weighted_maintainers, maintainer, BASE_REVIEWER_WEIGHT)
end
end
private
attr_reader :maintainers
def add_weighted_reviewer(reviewers, reviewer, weight)
if reviewer.reduced_capacity
reviewers.fill(reviewer, reviewers.size, weight)
else
reviewers.fill(reviewer, reviewers.size, weight * CAPACITY_MULTIPLIER)
end
end
end
end
end
end
# frozen_string_literal: true
require_relative '../weightage'
module Gitlab
module Danger
module Weightage
# Weights after (current multiplier of 2)
#
# +------------------------------+--------------------------------+
# | reviewer type | weight(times in reviewer pool) |
# +------------------------------+--------------------------------+
# | reduced capacity reviewer | 1 |
# | reviewer | 2 |
# | hungry reviewer | 4 |
# | reduced capacity traintainer | 3 |
# | traintainer | 6 |
# | hungry traintainer | 8 |
# +------------------------------+--------------------------------+
#
class Reviewers
DEFAULT_REVIEWER_WEIGHT = CAPACITY_MULTIPLIER * BASE_REVIEWER_WEIGHT
TRAINTAINER_WEIGHT = 3
def initialize(reviewers, traintainers)
@reviewers = reviewers
@traintainers = traintainers
end
def execute
# TODO: take CODEOWNERS into account?
# https://gitlab.com/gitlab-org/gitlab/issues/26723
weighted_reviewers + weighted_traintainers
end
private
attr_reader :reviewers, :traintainers
def weighted_reviewers
reviewers.each_with_object([]) do |reviewer, total_reviewers|
add_weighted_reviewer(total_reviewers, reviewer, BASE_REVIEWER_WEIGHT)
end
end
def weighted_traintainers
traintainers.each_with_object([]) do |reviewer, total_traintainers|
add_weighted_reviewer(total_traintainers, reviewer, TRAINTAINER_WEIGHT)
end
end
def add_weighted_reviewer(reviewers, reviewer, weight)
if reviewer.reduced_capacity
reviewers.fill(reviewer, reviewers.size, weight)
elsif reviewer.hungry
reviewers.fill(reviewer, reviewers.size, weight * CAPACITY_MULTIPLIER + DEFAULT_REVIEWER_WEIGHT)
else
reviewers.fill(reviewer, reviewers.size, weight * CAPACITY_MULTIPLIER)
end
end
end
end
end
end
......@@ -245,69 +245,6 @@ RSpec.describe Gitlab::Danger::Roulette do
end
end
end
describe 'reviewer suggestion probability' do
let(:reviewer) { teammate_with_capability('reviewer', 'reviewer backend') }
let(:hungry_reviewer) { teammate_with_capability('hungry_reviewer', 'reviewer backend', hungry: true) }
let(:traintainer) { teammate_with_capability('traintainer', 'trainee_maintainer backend') }
let(:hungry_traintainer) { teammate_with_capability('hungry_traintainer', 'trainee_maintainer backend', hungry: true) }
let(:teammates) do
[
reviewer.to_h,
hungry_reviewer.to_h,
traintainer.to_h,
hungry_traintainer.to_h
]
end
let(:categories) { [:backend] }
# This test is testing probability with inherent randomness.
# The variance is inversely related to sample size
# Given large enough sample size, the variance would be smaller,
# but the test would take longer.
# Given smaller sample size, the variance would be larger,
# but the test would take less time.
let!(:sample_size) { 500 }
let!(:variance) { 0.1 }
before do
# This test needs actual randomness to simulate probabilities
allow(subject).to receive(:new_random).and_return(Random.new)
WebMock
.stub_request(:get, described_class::ROULETTE_DATA_URL)
.to_return(body: teammate_json)
end
it 'has 1:2:3:4 probability of picking reviewer, hungry_reviewer, traintainer, hungry_traintainer' do
picks = Array.new(sample_size).map do
spins = subject.spin(project, categories, timezone_experiment: timezone_experiment)
spins.first.reviewer.name
end
expect(probability(picks, 'reviewer')).to be_within(variance).of(0.1)
expect(probability(picks, 'hungry_reviewer')).to be_within(variance).of(0.2)
expect(probability(picks, 'traintainer')).to be_within(variance).of(0.3)
expect(probability(picks, 'hungry_traintainer')).to be_within(variance).of(0.4)
end
def probability(picks, role)
picks.count(role).to_f / picks.length
end
def teammate_with_capability(name, capability, hungry: false)
Gitlab::Danger::Teammate.new(
{
'name' => name,
'projects' => {
'gitlab' => capability
},
'available' => true,
'hungry' => hungry
}
)
end
end
end
RSpec::Matchers.define :match_teammates do |expected|
......
# frozen_string_literal: true
require 'gitlab/danger/weightage/maintainers'
RSpec.describe Gitlab::Danger::Weightage::Maintainers do
let(:multiplier) { Gitlab::Danger::Weightage::CAPACITY_MULTIPLIER }
let(:regular_maintainer) { double('Teammate', reduced_capacity: false) }
let(:reduced_capacity_maintainer) { double('Teammate', reduced_capacity: true) }
let(:maintainers) do
[
regular_maintainer,
reduced_capacity_maintainer
]
end
let(:maintainer_count) { Gitlab::Danger::Weightage::BASE_REVIEWER_WEIGHT * multiplier }
let(:reduced_capacity_maintainer_count) { Gitlab::Danger::Weightage::BASE_REVIEWER_WEIGHT }
subject(:weighted_maintainers) { described_class.new(maintainers).execute }
describe '#execute' do
it 'weights the maintainers overall' do
expect(weighted_maintainers.count).to eq maintainer_count + reduced_capacity_maintainer_count
end
it 'has total count of regular maintainers' do
expect(weighted_maintainers.count { |r| r.object_id == regular_maintainer.object_id }).to eq maintainer_count
end
it 'has count of reduced capacity maintainers' do
expect(weighted_maintainers.count { |r| r.object_id == reduced_capacity_maintainer.object_id }).to eq reduced_capacity_maintainer_count
end
end
end
# frozen_string_literal: true
require 'gitlab/danger/weightage/reviewers'
RSpec.describe Gitlab::Danger::Weightage::Reviewers do
let(:multiplier) { Gitlab::Danger::Weightage::CAPACITY_MULTIPLIER }
let(:regular_reviewer) { double('Teammate', hungry: false, reduced_capacity: false) }
let(:hungry_reviewer) { double('Teammate', hungry: true, reduced_capacity: false) }
let(:reduced_capacity_reviewer) { double('Teammate', hungry: false, reduced_capacity: true) }
let(:reviewers) do
[
hungry_reviewer,
regular_reviewer,
reduced_capacity_reviewer
]
end
let(:regular_traintainer) { double('Teammate', hungry: false, reduced_capacity: false) }
let(:hungry_traintainer) { double('Teammate', hungry: true, reduced_capacity: false) }
let(:reduced_capacity_traintainer) { double('Teammate', hungry: false, reduced_capacity: true) }
let(:traintainers) do
[
hungry_traintainer,
regular_traintainer,
reduced_capacity_traintainer
]
end
let(:hungry_reviewer_count) { Gitlab::Danger::Weightage::BASE_REVIEWER_WEIGHT * multiplier + described_class::DEFAULT_REVIEWER_WEIGHT }
let(:hungry_traintainer_count) { described_class::TRAINTAINER_WEIGHT * multiplier + described_class::DEFAULT_REVIEWER_WEIGHT }
let(:reviewer_count) { Gitlab::Danger::Weightage::BASE_REVIEWER_WEIGHT * multiplier }
let(:traintainer_count) { Gitlab::Danger::Weightage::BASE_REVIEWER_WEIGHT * described_class::TRAINTAINER_WEIGHT * multiplier }
let(:reduced_capacity_reviewer_count) { Gitlab::Danger::Weightage::BASE_REVIEWER_WEIGHT }
let(:reduced_capacity_traintainer_count) { described_class::TRAINTAINER_WEIGHT }
subject(:weighted_reviewers) { described_class.new(reviewers, traintainers).execute }
describe '#execute', :aggregate_failures do
it 'weights the reviewers overall' do
reviewers_count = hungry_reviewer_count + reviewer_count + reduced_capacity_reviewer_count
traintainers_count = hungry_traintainer_count + traintainer_count + reduced_capacity_traintainer_count
expect(weighted_reviewers.count).to eq reviewers_count + traintainers_count
end
it 'has total count of hungry reviewers and traintainers' do
expect(weighted_reviewers.count(&:hungry)).to eq hungry_reviewer_count + hungry_traintainer_count
expect(weighted_reviewers.count { |r| r.object_id == hungry_reviewer.object_id }).to eq hungry_reviewer_count
expect(weighted_reviewers.count { |r| r.object_id == hungry_traintainer.object_id }).to eq hungry_traintainer_count
end
it 'has total count of regular reviewers and traintainers' do
expect(weighted_reviewers.count { |r| r.object_id == regular_reviewer.object_id }).to eq reviewer_count
expect(weighted_reviewers.count { |r| r.object_id == regular_traintainer.object_id }).to eq traintainer_count
end
it 'has count of reduced capacity reviewers' do
expect(weighted_reviewers.count(&:reduced_capacity)).to eq reduced_capacity_reviewer_count + reduced_capacity_traintainer_count
expect(weighted_reviewers.count { |r| r.object_id == reduced_capacity_reviewer.object_id }).to eq reduced_capacity_reviewer_count
expect(weighted_reviewers.count { |r| r.object_id == reduced_capacity_traintainer.object_id }).to eq reduced_capacity_traintainer_count
end
end
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