Extract roulette to its own module
So it's more modular and extensible
This commit is contained in:
Lin Jen-Shin
parent
c8b4edf651
commit
c90ba127bf
|
|
@ -1,5 +1,6 @@
|
|||
# frozen_string_literal: true
|
||||
danger.import_plugin('danger/plugins/helper.rb')
|
||||
danger.import_plugin('danger/plugins/roulette.rb')
|
||||
|
||||
unless helper.release_automation?
|
||||
danger.import_dangerfile(path: 'danger/metadata')
|
||||
|
|
|
|||
|
|
@ -1,8 +1,5 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
require 'net/http'
|
||||
require 'yaml'
|
||||
|
||||
require_relative '../../lib/gitlab/danger/helper'
|
||||
|
||||
module Danger
|
||||
|
|
|
|||
|
|
@ -0,0 +1,10 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
require_relative '../../lib/gitlab/danger/roulette'
|
||||
|
||||
module Danger
|
||||
class Roulette < Plugin
|
||||
# Put the helper code somewhere it can be tested
|
||||
include Gitlab::Danger::Roulette
|
||||
end
|
||||
end
|
||||
|
|
@ -32,7 +32,7 @@ for them.
|
|||
MARKDOWN
|
||||
|
||||
def spin_for_category(team, project, category, branch_name)
|
||||
rng = Random.new(Digest::MD5.hexdigest(branch_name).to_i(16))
|
||||
random = roulette.new_random(branch_name)
|
||||
|
||||
reviewers = team.select { |member| member.reviewer?(project, category) }
|
||||
traintainers = team.select { |member| member.traintainer?(project, category) }
|
||||
|
|
@ -42,43 +42,12 @@ def spin_for_category(team, project, category, branch_name)
|
|||
# https://gitlab.com/gitlab-org/gitlab-ce/issues/57653
|
||||
|
||||
# Make traintainers have triple the chance to be picked as a reviewer
|
||||
reviewer = spin_for_person(reviewers + traintainers + traintainers, random: rng)
|
||||
maintainer = spin_for_person(maintainers, random: rng)
|
||||
reviewer = roulette.spin_for_person(reviewers + traintainers + traintainers, random: random)
|
||||
maintainer = roulette.spin_for_person(maintainers, random: random)
|
||||
|
||||
"| #{helper.label_for_category(category)} | #{reviewer&.markdown_name} | #{maintainer&.markdown_name} |"
|
||||
end
|
||||
|
||||
# Known issue: If someone is rejected due to OOO, and then becomes not OOO, the
|
||||
# selection will change on next spin
|
||||
def spin_for_person(people, random:)
|
||||
person = nil
|
||||
people = people.dup
|
||||
|
||||
people.size.times do
|
||||
person = people.sample(random: random)
|
||||
|
||||
break person unless out_of_office?(person)
|
||||
|
||||
people -= [person]
|
||||
end
|
||||
|
||||
person
|
||||
end
|
||||
|
||||
def out_of_office?(person)
|
||||
username = CGI.escape(person.username)
|
||||
api_endpoint = "https://gitlab.com/api/v4/users/#{username}/status"
|
||||
response = HTTParty.get(api_endpoint) # rubocop:disable Gitlab/HTTParty
|
||||
|
||||
if response.code == 200
|
||||
response["message"]&.match(/OOO/i)
|
||||
else
|
||||
false # this is no worse than not checking for OOO
|
||||
end
|
||||
rescue
|
||||
false
|
||||
end
|
||||
|
||||
def build_list(items)
|
||||
list = items.map { |filename| "* `#{filename}`" }.join("\n")
|
||||
|
||||
|
|
@ -101,14 +70,12 @@ categories = changes.keys - [:unknown]
|
|||
# disable the review roulette for such MRs.
|
||||
if changes.any? && !gitlab.mr_labels.include?('single codebase') && !gitlab.mr_labels.include?('CSS cleanup')
|
||||
# Strip leading and trailing CE/EE markers
|
||||
canonical_branch_name = gitlab
|
||||
.mr_json['source_branch']
|
||||
.gsub(/^[ce]e-/, '')
|
||||
.gsub(/-[ce]e$/, '')
|
||||
canonical_branch_name =
|
||||
roulette.canonical_branch_name(gitlab.mr_json['source_branch'])
|
||||
|
||||
team =
|
||||
begin
|
||||
helper.project_team
|
||||
roulette.project_team(helper.project_name)
|
||||
rescue => err
|
||||
warn("Reviewer roulette failed to load team data: #{err.message}")
|
||||
[]
|
||||
|
|
|
|||
|
|
@ -1,6 +1,4 @@
|
|||
# frozen_string_literal: true
|
||||
require 'net/http'
|
||||
require 'json'
|
||||
|
||||
require_relative 'teammate'
|
||||
|
||||
|
|
@ -8,7 +6,6 @@ module Gitlab
|
|||
module Danger
|
||||
module Helper
|
||||
RELEASE_TOOLS_BOT = 'gitlab-release-tools-bot'
|
||||
ROULETTE_DATA_URL = URI.parse('https://about.gitlab.com/roulette.json').freeze
|
||||
|
||||
# Returns a list of all files that have been added, modified or renamed.
|
||||
# `git.modified_files` might contain paths that already have been renamed,
|
||||
|
|
@ -49,32 +46,6 @@ module Gitlab
|
|||
ee? ? 'gitlab-ee' : 'gitlab-ce'
|
||||
end
|
||||
|
||||
# Looks up the current list of GitLab team members and parses it into a
|
||||
# useful form
|
||||
#
|
||||
# @return [Array<Teammate>]
|
||||
def team
|
||||
@team ||=
|
||||
begin
|
||||
rsp = Net::HTTP.get_response(ROULETTE_DATA_URL)
|
||||
raise "Failed to read #{ROULETTE_DATA_URL}: #{rsp.code} #{rsp.message}" unless
|
||||
rsp.is_a?(Net::HTTPSuccess)
|
||||
|
||||
data = JSON.parse(rsp.body)
|
||||
data.map { |hash| ::Gitlab::Danger::Teammate.new(hash) }
|
||||
rescue JSON::ParserError
|
||||
raise "Failed to parse JSON response from #{ROULETTE_DATA_URL}"
|
||||
end
|
||||
end
|
||||
|
||||
# Like +team+, but only returns teammates in the current project, based on
|
||||
# project_name.
|
||||
#
|
||||
# @return [Array<Teammate>]
|
||||
def project_team
|
||||
team.select { |member| member.in_project?(project_name) }
|
||||
end
|
||||
|
||||
# @return [Hash<String,Array<String>>]
|
||||
def changes_by_category
|
||||
all_changed_files.each_with_object(Hash.new { |h, k| h[k] = [] }) do |file, hash|
|
||||
|
|
|
|||
|
|
@ -0,0 +1,84 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
require 'net/http'
|
||||
require 'json'
|
||||
require 'cgi'
|
||||
|
||||
require_relative 'teammate'
|
||||
|
||||
module Gitlab
|
||||
module Danger
|
||||
module Roulette
|
||||
ROULETTE_DATA_URL = 'https://about.gitlab.com/roulette.json'
|
||||
HTTPError = Class.new(RuntimeError)
|
||||
|
||||
# Looks up the current list of GitLab team members and parses it into a
|
||||
# useful form
|
||||
#
|
||||
# @return [Array<Teammate>]
|
||||
def team
|
||||
@team ||=
|
||||
begin
|
||||
data = http_get_json(ROULETTE_DATA_URL)
|
||||
data.map { |hash| ::Gitlab::Danger::Teammate.new(hash) }
|
||||
rescue JSON::ParserError
|
||||
raise "Failed to parse JSON response from #{ROULETTE_DATA_URL}"
|
||||
end
|
||||
end
|
||||
|
||||
# Like +team+, but only returns teammates in the current project, based on
|
||||
# project_name.
|
||||
#
|
||||
# @return [Array<Teammate>]
|
||||
def project_team(project_name)
|
||||
team.select { |member| member.in_project?(project_name) }
|
||||
end
|
||||
|
||||
def canonical_branch_name(branch_name)
|
||||
branch_name.gsub(/^[ce]e-|-[ce]e$/, '')
|
||||
end
|
||||
|
||||
def new_random(seed)
|
||||
Random.new(Digest::MD5.hexdigest(seed).to_i(16))
|
||||
end
|
||||
|
||||
# Known issue: If someone is rejected due to OOO, and then becomes not OOO, the
|
||||
# selection will change on next spin
|
||||
def spin_for_person(people, random:)
|
||||
person = nil
|
||||
people = people.dup
|
||||
|
||||
people.size.times do
|
||||
person = people.sample(random: random)
|
||||
|
||||
break person unless out_of_office?(person)
|
||||
|
||||
people -= [person]
|
||||
end
|
||||
|
||||
person
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def out_of_office?(person)
|
||||
username = CGI.escape(person.username)
|
||||
api_endpoint = "https://gitlab.com/api/v4/users/#{username}/status"
|
||||
response = http_get_json(api_endpoint)
|
||||
response["message"]&.match?(/OOO/i)
|
||||
rescue HTTPError, JSON::ParserError
|
||||
false # this is no worse than not checking for OOO
|
||||
end
|
||||
|
||||
def http_get_json(url)
|
||||
rsp = Net::HTTP.get_response(URI.parse(url))
|
||||
|
||||
unless rsp.is_a?(Net::HTTPSuccess)
|
||||
raise HTTPError, "Failed to read #{url}: #{rsp.code} #{rsp.message}"
|
||||
end
|
||||
|
||||
JSON.parse(rsp.body)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
@ -2,7 +2,6 @@
|
|||
|
||||
require 'fast_spec_helper'
|
||||
require 'rspec-parameterized'
|
||||
require 'webmock/rspec'
|
||||
|
||||
require 'gitlab/danger/helper'
|
||||
|
||||
|
|
@ -19,39 +18,6 @@ describe Gitlab::Danger::Helper do
|
|||
end
|
||||
end
|
||||
|
||||
let(:teammate_json) do
|
||||
<<~JSON
|
||||
[
|
||||
{
|
||||
"username": "in-gitlab-ce",
|
||||
"name": "CE maintainer",
|
||||
"projects":{ "gitlab-ce": "maintainer backend" }
|
||||
},
|
||||
{
|
||||
"username": "in-gitlab-ee",
|
||||
"name": "EE reviewer",
|
||||
"projects":{ "gitlab-ee": "reviewer frontend" }
|
||||
}
|
||||
]
|
||||
JSON
|
||||
end
|
||||
|
||||
let(:ce_teammate_matcher) do
|
||||
satisfy do |teammate|
|
||||
teammate.username == 'in-gitlab-ce' &&
|
||||
teammate.name == 'CE maintainer' &&
|
||||
teammate.projects == { 'gitlab-ce' => 'maintainer backend' }
|
||||
end
|
||||
end
|
||||
|
||||
let(:ee_teammate_matcher) do
|
||||
satisfy do |teammate|
|
||||
teammate.username == 'in-gitlab-ee' &&
|
||||
teammate.name == 'EE reviewer' &&
|
||||
teammate.projects == { 'gitlab-ee' => 'reviewer frontend' }
|
||||
end
|
||||
end
|
||||
|
||||
let(:fake_git) { double('fake-git') }
|
||||
|
||||
subject(:helper) { FakeDanger.new(git: fake_git) }
|
||||
|
|
@ -119,69 +85,6 @@ describe Gitlab::Danger::Helper do
|
|||
end
|
||||
end
|
||||
|
||||
describe '#team' do
|
||||
subject(:team) { helper.team }
|
||||
|
||||
context 'HTTP failure' do
|
||||
before do
|
||||
WebMock
|
||||
.stub_request(:get, 'https://about.gitlab.com/roulette.json')
|
||||
.to_return(status: 404)
|
||||
end
|
||||
|
||||
it 'raises a pretty error' do
|
||||
expect { team }.to raise_error(/Failed to read/)
|
||||
end
|
||||
end
|
||||
|
||||
context 'JSON failure' do
|
||||
before do
|
||||
WebMock
|
||||
.stub_request(:get, 'https://about.gitlab.com/roulette.json')
|
||||
.to_return(body: 'INVALID JSON')
|
||||
end
|
||||
|
||||
it 'raises a pretty error' do
|
||||
expect { team }.to raise_error(/Failed to parse/)
|
||||
end
|
||||
end
|
||||
|
||||
context 'success' do
|
||||
before do
|
||||
WebMock
|
||||
.stub_request(:get, 'https://about.gitlab.com/roulette.json')
|
||||
.to_return(body: teammate_json)
|
||||
end
|
||||
|
||||
it 'returns an array of teammates' do
|
||||
is_expected.to contain_exactly(ce_teammate_matcher, ee_teammate_matcher)
|
||||
end
|
||||
|
||||
it 'memoizes the result' do
|
||||
expect(team.object_id).to eq(helper.team.object_id)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#project_team' do
|
||||
subject { helper.project_team }
|
||||
|
||||
before do
|
||||
WebMock
|
||||
.stub_request(:get, 'https://about.gitlab.com/roulette.json')
|
||||
.to_return(body: teammate_json)
|
||||
end
|
||||
|
||||
it 'filters team by project_name' do
|
||||
expect(helper)
|
||||
.to receive(:project_name)
|
||||
.at_least(:once)
|
||||
.and_return('gitlab-ce')
|
||||
|
||||
is_expected.to contain_exactly(ce_teammate_matcher)
|
||||
end
|
||||
end
|
||||
|
||||
describe '#changes_by_category' do
|
||||
it 'categorizes changed files' do
|
||||
expect(fake_git).to receive(:added_files) { %w[foo foo.md foo.rb foo.js db/foo qa/foo ee/changelogs/foo.yml] }
|
||||
|
|
|
|||
|
|
@ -0,0 +1,101 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
require 'fast_spec_helper'
|
||||
require 'webmock/rspec'
|
||||
|
||||
require 'gitlab/danger/roulette'
|
||||
|
||||
describe Gitlab::Danger::Roulette do
|
||||
let(:teammate_json) do
|
||||
<<~JSON
|
||||
[
|
||||
{
|
||||
"username": "in-gitlab-ce",
|
||||
"name": "CE maintainer",
|
||||
"projects":{ "gitlab-ce": "maintainer backend" }
|
||||
},
|
||||
{
|
||||
"username": "in-gitlab-ee",
|
||||
"name": "EE reviewer",
|
||||
"projects":{ "gitlab-ee": "reviewer frontend" }
|
||||
}
|
||||
]
|
||||
JSON
|
||||
end
|
||||
|
||||
let(:ce_teammate_matcher) do
|
||||
satisfy do |teammate|
|
||||
teammate.username == 'in-gitlab-ce' &&
|
||||
teammate.name == 'CE maintainer' &&
|
||||
teammate.projects == { 'gitlab-ce' => 'maintainer backend' }
|
||||
end
|
||||
end
|
||||
|
||||
let(:ee_teammate_matcher) do
|
||||
satisfy do |teammate|
|
||||
teammate.username == 'in-gitlab-ee' &&
|
||||
teammate.name == 'EE reviewer' &&
|
||||
teammate.projects == { 'gitlab-ee' => 'reviewer frontend' }
|
||||
end
|
||||
end
|
||||
|
||||
subject(:roulette) { Object.new.extend(described_class) }
|
||||
|
||||
describe '#team' do
|
||||
subject(:team) { roulette.team }
|
||||
|
||||
context 'HTTP failure' do
|
||||
before do
|
||||
WebMock
|
||||
.stub_request(:get, described_class::ROULETTE_DATA_URL)
|
||||
.to_return(status: 404)
|
||||
end
|
||||
|
||||
it 'raises a pretty error' do
|
||||
expect { team }.to raise_error(/Failed to read/)
|
||||
end
|
||||
end
|
||||
|
||||
context 'JSON failure' do
|
||||
before do
|
||||
WebMock
|
||||
.stub_request(:get, described_class::ROULETTE_DATA_URL)
|
||||
.to_return(body: 'INVALID JSON')
|
||||
end
|
||||
|
||||
it 'raises a pretty error' do
|
||||
expect { team }.to raise_error(/Failed to parse/)
|
||||
end
|
||||
end
|
||||
|
||||
context 'success' do
|
||||
before do
|
||||
WebMock
|
||||
.stub_request(:get, described_class::ROULETTE_DATA_URL)
|
||||
.to_return(body: teammate_json)
|
||||
end
|
||||
|
||||
it 'returns an array of teammates' do
|
||||
is_expected.to contain_exactly(ce_teammate_matcher, ee_teammate_matcher)
|
||||
end
|
||||
|
||||
it 'memoizes the result' do
|
||||
expect(team.object_id).to eq(roulette.team.object_id)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#project_team' do
|
||||
subject { roulette.project_team('gitlab-ce') }
|
||||
|
||||
before do
|
||||
WebMock
|
||||
.stub_request(:get, described_class::ROULETTE_DATA_URL)
|
||||
.to_return(body: teammate_json)
|
||||
end
|
||||
|
||||
it 'filters team by project_name' do
|
||||
is_expected.to contain_exactly(ce_teammate_matcher)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
@ -1,5 +1,9 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
require 'fast_spec_helper'
|
||||
|
||||
require 'gitlab/danger/teammate'
|
||||
|
||||
describe Gitlab::Danger::Teammate do
|
||||
subject { described_class.new({ 'projects' => projects }) }
|
||||
let(:projects) { { project => capabilities } }
|
||||
|
|
@ -9,15 +13,15 @@ describe Gitlab::Danger::Teammate do
|
|||
let(:capabilities) { ['reviewer backend', 'maintainer frontend', 'trainee_maintainer database'] }
|
||||
|
||||
it '#reviewer? supports multiple roles per project' do
|
||||
expect(subject.reviewer?(project, 'backend')).to be_truthy
|
||||
expect(subject.reviewer?(project, :backend)).to be_truthy
|
||||
end
|
||||
|
||||
it '#traintainer? supports multiple roles per project' do
|
||||
expect(subject.traintainer?(project, 'database')).to be_truthy
|
||||
expect(subject.traintainer?(project, :database)).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)).to be_truthy
|
||||
end
|
||||
end
|
||||
|
||||
|
|
@ -25,15 +29,15 @@ describe Gitlab::Danger::Teammate 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)).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)).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)).to be_falsey
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
Loading…
Reference in New Issue