Commit c3334903 authored by Rémy Coutable's avatar Rémy Coutable

Merge branch '243712-consider-hungry-people-in-review-roulette' into 'master'

Assign reviews to hungry reviewers first

Closes #243712

See merge request gitlab-org/gitlab!41938
parents 8b77146a 5d5165dd
......@@ -146,13 +146,18 @@ 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 }
# TODO: take CODEOWNERS into account?
# https://gitlab.com/gitlab-org/gitlab/issues/26723
# Make traintainers have triple the chance to be picked as a reviewer
random = new_random(mr_source_branch)
reviewer = spin_for_person(reviewers + traintainers + traintainers, random: random, timezone_experiment: timezone_experiment)
# 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
reviewer = spin_for_person(weighted_reviewers, random: random, timezone_experiment: timezone_experiment)
maintainer = spin_for_person(maintainers, random: random, timezone_experiment: timezone_experiment)
Spin.new(category, reviewer, maintainer, false, timezone_experiment)
......
......@@ -3,7 +3,7 @@
module Gitlab
module Danger
class Teammate
attr_reader :options, :username, :name, :role, :projects, :available, :tz_offset_hours
attr_reader :options, :username, :name, :role, :projects, :available, :hungry, :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 = {})
......@@ -14,6 +14,7 @@ module Gitlab
@role = options['role']
@projects = options['projects']
@available = options['available']
@hungry = options['hungry']
@tz_offset_hours = options['tz_offset_hours']
end
......
......@@ -67,14 +67,18 @@ RSpec.describe Gitlab::Danger::Roulette do
)
end
let(:teammate_json) do
let(:teammates) do
[
backend_maintainer.to_h,
frontend_maintainer.to_h,
frontend_reviewer.to_h,
software_engineer_in_test.to_h,
engineering_productivity_reviewer.to_h
].to_json
]
end
let(:teammate_json) do
teammates.to_json
end
subject(:roulette) { Object.new.extend(described_class) }
......@@ -210,6 +214,69 @@ 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|
......
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