Merge branch 'jj-move-round-robin' into 'master'

Move RoundRobin into applicable experiment file

See merge request gitlab-org/gitlab!56438

app/experiments/members/invite_email_experiment.rb

@@ -8,7 +8,82 @@ module Members
     INVITE_TYPE = 'initial_email'
 
     def resolve_variant_name
-      Strategy::RoundRobin.new(feature_flag_name, %i[avatar permission_info control]).execute
+      RoundRobin.new(feature_flag_name, %i[avatar permission_info control]).execute
+    end
+  end
+
+  class RoundRobin
+    CacheError = Class.new(StandardError)
+
+    COUNTER_EXPIRE_TIME = 86400 # one day
+
+    def initialize(key, variants)
+      @key = key
+      @variants = variants
+    end
+
+    def execute
+      increment_counter
+      resolve_variant_name
+    end
+
+    # When the counter would expire
+    #
+    # @api private Used internally by SRE and debugging purpose
+    # @return [Integer] Number in seconds until expiration or false if never
+    def counter_expires_in
+      Gitlab::Redis::SharedState.with do |redis|
+        redis.ttl(key)
+      end
+    end
+
+    # Return the actual counter value
+    #
+    # @return [Integer] value
+    def counter_value
+      Gitlab::Redis::SharedState.with do |redis|
+        (redis.get(key) || 0).to_i
+      end
+    end
+
+    # Reset the counter
+    #
+    # @private Used internally by SRE and debugging purpose
+    # @return [Boolean] whether reset was a success
+    def reset!
+      redis_cmd do |redis|
+        redis.del(key)
+      end
+    end
+
+    private
+
+    attr_reader :key, :variants
+
+    # Increase the counter
+    #
+    # @return [Boolean] whether operation was a success
+    def increment_counter
+      redis_cmd do |redis|
+        redis.incr(key)
+        redis.expire(key, COUNTER_EXPIRE_TIME)
+      end
+    end
+
+    def resolve_variant_name
+      remainder = counter_value % variants.size
+
+      variants[remainder]
+    end
+
+    def redis_cmd
+      Gitlab::Redis::SharedState.with { |redis| yield(redis) }
+
+      true
+    rescue CacheError => e
+      Gitlab::AppLogger.warn("GitLab: An unexpected error occurred in writing to Redis: #{e}")
+
+      false
     end
   end
 end

app/experiments/strategy/round_robin.rb

-# frozen_string_literal: true
-
-module Strategy
-  class RoundRobin
-    CacheError = Class.new(StandardError)
-
-    COUNTER_EXPIRE_TIME = 86400 # one day
-
-    def initialize(key, variants)
-      @key = key
-      @variants = variants
-    end
-
-    def execute
-      increment_counter
-      resolve_variant_name
-    end
-
-    # When the counter would expire
-    #
-    # @api private Used internally by SRE and debugging purpose
-    # @return [Integer] Number in seconds until expiration or false if never
-    def counter_expires_in
-      Gitlab::Redis::SharedState.with do |redis|
-        redis.ttl(key)
-      end
-    end
-
-    # Return the actual counter value
-    #
-    # @return [Integer] value
-    def counter_value
-      Gitlab::Redis::SharedState.with do |redis|
-        (redis.get(key) || 0).to_i
-      end
-    end
-
-    # Reset the counter
-    #
-    # @private Used internally by SRE and debugging purpose
-    # @return [Boolean] whether reset was a success
-    def reset!
-      redis_cmd do |redis|
-        redis.del(key)
-      end
-    end
-
-    private
-
-    attr_reader :key, :variants
-
-    # Increase the counter
-    #
-    # @return [Boolean] whether operation was a success
-    def increment_counter
-      redis_cmd do |redis|
-        redis.incr(key)
-        redis.expire(key, COUNTER_EXPIRE_TIME)
-      end
-    end
-
-    def resolve_variant_name
-      remainder = counter_value % variants.size
-
-      variants[remainder]
-    end
-
-    def redis_cmd
-      Gitlab::Redis::SharedState.with { |redis| yield(redis) }
-
-      true
-    rescue CacheError => e
-      Gitlab::AppLogger.warn("GitLab: An unexpected error occurred in writing to Redis: #{e}")
-
-      false
-    end
-  end
-end

spec/experiments/members/invite_email_experiment_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Members::InviteEmailExperiment do
+RSpec.describe Members::InviteEmailExperiment, :clean_gitlab_redis_shared_state do
   subject(:invite_email) { experiment('members/invite_email', **context) }
 
   let(:context) { { actor: double('Member', created_by: double('User', avatar_url: '_avatar_url_')) } }
@@ -23,7 +23,7 @@ RSpec.describe Members::InviteEmailExperiment do
     end
   end
 
-  describe "variant resolution", :clean_gitlab_redis_shared_state do
+  describe "variant resolution" do
     it "proves out round robin in variant selection", :aggregate_failures do
       instance_1 = described_class.new('members/invite_email', **context)
       allow(instance_1).to receive(:enabled?).and_return(true)
@@ -45,4 +45,69 @@ RSpec.describe Members::InviteEmailExperiment do
       expect(instance_3.variant.name).to eq('avatar')
     end
   end
+
+  describe Members::RoundRobin do
+    subject(:round_robin) { Members::RoundRobin.new('_key_', %i[variant1 variant2]) }
+
+    describe "execute" do
+      context "when there are 2 variants" do
+        it "proves out round robin in selection", :aggregate_failures do
+          expect(round_robin.execute).to eq :variant2
+          expect(round_robin.execute).to eq :variant1
+          expect(round_robin.execute).to eq :variant2
+        end
+      end
+
+      context "when there are more than 2 variants" do
+        subject(:round_robin) { Members::RoundRobin.new('_key_', %i[variant1 variant2 variant3]) }
+
+        it "proves out round robin in selection", :aggregate_failures do
+          expect(round_robin.execute).to eq :variant2
+          expect(round_robin.execute).to eq :variant3
+          expect(round_robin.execute).to eq :variant1
+
+          expect(round_robin.execute).to eq :variant2
+          expect(round_robin.execute).to eq :variant3
+          expect(round_robin.execute).to eq :variant1
+        end
+      end
+
+      context "when writing to cache fails" do
+        subject(:round_robin) { Members::RoundRobin.new('_key_', []) }
+
+        it "raises an error and logs" do
+          allow(Gitlab::Redis::SharedState).to receive(:with).and_raise(Members::RoundRobin::CacheError)
+          expect(Gitlab::AppLogger).to receive(:warn)
+
+          expect { round_robin.execute }.to raise_error(Members::RoundRobin::CacheError)
+        end
+      end
+    end
+
+    describe "#counter_expires_in" do
+      it 'displays the expiration time in seconds' do
+        round_robin.execute
+
+        expect(round_robin.counter_expires_in).to be_between(0, described_class::COUNTER_EXPIRE_TIME)
+      end
+    end
+
+    describe '#value' do
+      it 'get the count' do
+        expect(round_robin.counter_value).to eq(0)
+
+        round_robin.execute
+
+        expect(round_robin.counter_value).to eq(1)
+      end
+    end
+
+    describe '#reset!' do
+      it 'resets the count down to zero' do
+        3.times { round_robin.execute }
+
+        expect { round_robin.reset! }.to change { round_robin.counter_value }.from(3).to(0)
+      end
+    end
+  end
 end

spec/experiments/strategy/round_robin_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Strategy::RoundRobin, :clean_gitlab_redis_shared_state do
-  subject(:round_robin) { described_class.new('_key_', %i[variant1 variant2]) }
-
-  describe "execute" do
-    context "when there are 2 variants" do
-      it "proves out round robin in selection", :aggregate_failures do
-        expect(round_robin.execute).to eq :variant2
-        expect(round_robin.execute).to eq :variant1
-        expect(round_robin.execute).to eq :variant2
-      end
-    end
-
-    context "when there are more than 2 variants" do
-      subject(:round_robin) { described_class.new('_key_', %i[variant1 variant2 variant3]) }
-
-      it "proves out round robin in selection", :aggregate_failures do
-        expect(round_robin.execute).to eq :variant2
-        expect(round_robin.execute).to eq :variant3
-        expect(round_robin.execute).to eq :variant1
-
-        expect(round_robin.execute).to eq :variant2
-        expect(round_robin.execute).to eq :variant3
-        expect(round_robin.execute).to eq :variant1
-      end
-    end
-
-    context "when writing to cache fails" do
-      subject(:round_robin) { described_class.new('_key_', []) }
-
-      it "raises an error and logs" do
-        allow(Gitlab::Redis::SharedState).to receive(:with).and_raise(Strategy::RoundRobin::CacheError)
-        expect(Gitlab::AppLogger).to receive(:warn)
-
-        expect { round_robin.execute }.to raise_error(Strategy::RoundRobin::CacheError)
-      end
-    end
-  end
-
-  describe "#counter_expires_in" do
-    it 'displays the expiration time in seconds' do
-      round_robin.execute
-
-      expect(round_robin.counter_expires_in).to be_between(0, described_class::COUNTER_EXPIRE_TIME)
-    end
-  end
-
-  describe '#value' do
-    it 'get the count' do
-      expect(round_robin.counter_value).to eq(0)
-
-      round_robin.execute
-
-      expect(round_robin.counter_value).to eq(1)
-    end
-  end
-
-  describe '#reset!' do
-    it 'resets the count down to zero' do
-      3.times { round_robin.execute }
-
-      expect { round_robin.reset! }.to change { round_robin.counter_value }.from(3).to(0)
-    end
-  end
-end