Merge branch '292968-introduce-reduced-capacity-danger-roulette-functionality' into 'master'

Introduce reduced capacity danger roulette functionality

See merge request gitlab-org/gitlab!50172

lib/gitlab/danger/roulette.rb

@@ -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

lib/gitlab/danger/teammate.rb

@@ -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
 

lib/gitlab/danger/weightage.rb

+# 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

lib/gitlab/danger/weightage/maintainers.rb

+# 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

lib/gitlab/danger/weightage/reviewers.rb

+# 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

spec/lib/gitlab/danger/roulette_spec.rb

@@ -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|

spec/lib/gitlab/danger/weightage/maintainers_spec.rb

+# 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

spec/lib/gitlab/danger/weightage/reviewers_spec.rb

+# 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