Add latest changes from gitlab-org/gitlab@13-9-stable-eev13.9.0-rc42

5 files changed, 327 insertions, 30 deletions

diff --git a/spec/experiments/application_experiment/cache_spec.rb b/spec/experiments/application_experiment/cache_spec.rb
index a420d557155..4caa91e6ac4 100644
--- a/spec/experiments/application_experiment/cache_spec.rb
+++ b/spec/experiments/application_experiment/cache_spec.rb
@@ -51,16 +51,4 @@ RSpec.describe ApplicationExperiment::Cache do
       'invalid call to clear a non-hash cache key'
     )
   end
-
-  context "when the :caching_experiments feature is disabled" do
-    before do
-      stub_feature_flags(caching_experiments: false)
-    end
-
-    it "doesn't write to the cache" do
-      subject.write(key_field, 'value')
-
-      expect(subject.read(key_field)).to be_nil
-    end
-  end
 end
diff --git a/spec/experiments/application_experiment_spec.rb b/spec/experiments/application_experiment_spec.rb
index ece52d37351..501d344e920 100644
--- a/spec/experiments/application_experiment_spec.rb
+++ b/spec/experiments/application_experiment_spec.rb
@@ -2,8 +2,60 @@
 
 require 'spec_helper'
 
-RSpec.describe ApplicationExperiment do
-  subject { described_class.new(:stub) }
+RSpec.describe ApplicationExperiment, :experiment do
+  subject { described_class.new('namespaced/stub') }
+
+  let(:feature_definition) do
+    { name: 'namespaced_stub', type: 'experiment', group: 'group::adoption', default_enabled: false }
+  end
+
+  around do |example|
+    Feature::Definition.definitions[:namespaced_stub] = Feature::Definition.new('namespaced_stub.yml', feature_definition)
+    example.run
+    Feature::Definition.definitions.delete(:namespaced_stub)
+  end
+
+  before do
+    allow(subject).to receive(:enabled?).and_return(true)
+  end
+
+  it "naively assumes a 1x1 relationship to feature flags for tests" do
+    expect(Feature).to receive(:persist_used!).with('namespaced_stub')
+
+    described_class.new('namespaced/stub')
+  end
+
+  describe "enabled" do
+    before do
+      allow(subject).to receive(:enabled?).and_call_original
+
+      allow(Feature::Definition).to receive(:get).and_return('_instance_')
+      allow(Gitlab).to receive(:dev_env_or_com?).and_return(true)
+      allow(Feature).to receive(:get).and_return(double(state: :on))
+    end
+
+    it "is enabled when all criteria are met" do
+      expect(subject).to be_enabled
+    end
+
+    it "isn't enabled if the feature definition doesn't exist" do
+      expect(Feature::Definition).to receive(:get).with('namespaced_stub').and_return(nil)
+
+      expect(subject).not_to be_enabled
+    end
+
+    it "isn't enabled if we're not in dev or dotcom environments" do
+      expect(Gitlab).to receive(:dev_env_or_com?).and_return(false)
+
+      expect(subject).not_to be_enabled
+    end
+
+    it "isn't enabled if the feature flag state is :off" do
+      expect(Feature).to receive(:get).with('namespaced_stub').and_return(double(state: :off))
+
+      expect(subject).not_to be_enabled
+    end
+  end
 
   describe "publishing results" do
     it "tracks the assignment" do
@@ -16,9 +68,9 @@ RSpec.describe ApplicationExperiment do
       expect(Gon.global).to receive(:push).with(
         {
           experiment: {
-            'stub' => { # string key because it can be namespaced
-              experiment: 'stub',
-              key: 'e8f65fd8d973f9985dc7ea3cf1614ae1',
+            'namespaced/stub' => { # string key because it can be namespaced
+              experiment: 'namespaced/stub',
+              key: '86208ac54ca798e11f127e8b23ec396a',
               variant: 'control'
             }
           }
@@ -31,8 +83,8 @@ RSpec.describe ApplicationExperiment do
   end
 
   describe "tracking events", :snowplow do
-    it "doesn't track if excluded" do
-      subject.exclude { true }
+    it "doesn't track if we shouldn't track" do
+      allow(subject).to receive(:should_track?).and_return(false)
 
       subject.track(:action)
 
@@ -45,7 +97,7 @@ RSpec.describe ApplicationExperiment do
       ])
 
       expect_snowplow_event(
-        category: 'stub',
+        category: 'namespaced/stub',
         action: 'action',
         property: '_property_',
         context: [
@@ -55,7 +107,7 @@ RSpec.describe ApplicationExperiment do
           },
           {
             schema: 'iglu:com.gitlab/gitlab_experiment/jsonschema/0-3-0',
-            data: { experiment: 'stub', key: 'e8f65fd8d973f9985dc7ea3cf1614ae1', variant: 'control' }
+            data: { experiment: 'namespaced/stub', key: '86208ac54ca798e11f127e8b23ec396a', variant: 'control' }
           }
         ]
       )
@@ -63,18 +115,77 @@ RSpec.describe ApplicationExperiment do
   end
 
   describe "variant resolution" do
-    it "returns nil when not rolled out" do
-      stub_feature_flags(stub: false)
+    context "when using the default feature flag percentage rollout" do
+      it "uses the default value as specified in the yaml" do
+        expect(Feature).to receive(:enabled?).with('namespaced_stub', subject, type: :experiment, default_enabled: :yaml)
+
+        expect(subject.variant.name).to eq('control')
+      end
+
+      it "returns nil when not rolled out" do
+        stub_feature_flags(namespaced_stub: false)
+
+        expect(subject.variant.name).to eq('control')
+      end
+
+      context "when rolled out to 100%" do
+        it "returns the first variant name" do
+          subject.try(:variant1) {}
+          subject.try(:variant2) {}
 
-      expect(subject.variant.name).to eq('control')
+          expect(subject.variant.name).to eq('variant1')
+        end
+      end
     end
 
-    context "when rolled out to 100%" do
-      it "returns the first variant name" do
-        subject.try(:variant1) {}
-        subject.try(:variant2) {}
+    context "when using the round_robin strategy", :clean_gitlab_redis_shared_state do
+      context "when variants aren't supplied" do
+        subject :inheriting_class do
+          Class.new(described_class) do
+            def rollout_strategy
+              :round_robin
+            end
+          end.new('namespaced/stub')
+        end
+
+        it "raises an error" do
+          expect { inheriting_class.variants }.to raise_error(NotImplementedError)
+        end
+      end
+
+      context "when variants are supplied" do
+        let(:inheriting_class) do
+          Class.new(described_class) do
+            def rollout_strategy
+              :round_robin
+            end
+
+            def variants
+              %i[variant1 variant2 control]
+            end
+          end
+        end
+
+        it "proves out round robin in variant selection", :aggregate_failures do
+          instance_1 = inheriting_class.new('namespaced/stub')
+          allow(instance_1).to receive(:enabled?).and_return(true)
+          instance_2 = inheriting_class.new('namespaced/stub')
+          allow(instance_2).to receive(:enabled?).and_return(true)
+          instance_3 = inheriting_class.new('namespaced/stub')
+          allow(instance_3).to receive(:enabled?).and_return(true)
+
+          instance_1.try {}
+
+          expect(instance_1.variant.name).to eq('variant2')
+
+          instance_2.try {}
+
+          expect(instance_2.variant.name).to eq('control')
+
+          instance_3.try {}
 
-        expect(subject.variant.name).to eq('variant1')
+          expect(instance_3.variant.name).to eq('variant1')
+        end
       end
     end
   end
@@ -105,7 +216,7 @@ RSpec.describe ApplicationExperiment do
       # every control variant assigned, we'd inflate the cache size and
       # wouldn't be able to roll out to subjects that we'd already assigned to
       # the control.
-      stub_feature_flags(stub: false) # simulate being not rolled out
+      stub_feature_flags(namespaced_stub: false) # simulate being not rolled out
 
       expect(subject.variant.name).to eq('control') # if we ask, it should be control
 
diff --git a/spec/experiments/members/invite_email_experiment_spec.rb b/spec/experiments/members/invite_email_experiment_spec.rb
new file mode 100644
index 00000000000..4376c021385
--- /dev/null
+++ b/spec/experiments/members/invite_email_experiment_spec.rb
@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Members::InviteEmailExperiment do
+  subject :invite_email do
+    experiment('members/invite_email', actor: double('Member', created_by: double('User', avatar_url: '_avatar_url_')))
+  end
+
+  before do
+    allow(invite_email).to receive(:enabled?).and_return(true)
+  end
+
+  describe "#rollout_strategy" do
+    it "resolves to round_robin" do
+      expect(invite_email.rollout_strategy).to eq(:round_robin)
+    end
+  end
+
+  describe "#variants" do
+    it "has all the expected variants" do
+      expect(invite_email.variants).to match(%i[avatar permission_info control])
+    end
+  end
+
+  describe "exclusions", :experiment do
+    it "excludes when created by is nil" do
+      expect(experiment('members/invite_email')).to exclude(actor: double(created_by: nil))
+    end
+
+    it "excludes when avatar_url is nil" do
+      member_without_avatar_url = double('Member', created_by: double('User', avatar_url: nil))
+
+      expect(experiment('members/invite_email')).to exclude(actor: member_without_avatar_url)
+    end
+  end
+end
diff --git a/spec/experiments/new_project_readme_experiment_spec.rb b/spec/experiments/new_project_readme_experiment_spec.rb
new file mode 100644
index 00000000000..17e28cf6e7f
--- /dev/null
+++ b/spec/experiments/new_project_readme_experiment_spec.rb
@@ -0,0 +1,93 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe NewProjectReadmeExperiment, :experiment do
+  subject { described_class.new(actor: actor) }
+
+  let(:actor) { User.new(id: 42, created_at: Time.current) }
+
+  before do
+    stub_experiments(new_project_readme: :control)
+  end
+
+  describe "exclusions" do
+    let(:threshold) { described_class::MAX_ACCOUNT_AGE }
+
+    it { is_expected.to exclude(actor: User.new(created_at: (threshold + 1.minute).ago)) }
+    it { is_expected.not_to exclude(actor: User.new(created_at: (threshold - 1.minute).ago)) }
+  end
+
+  describe "the control behavior" do
+    subject { described_class.new(actor: actor).run(:control) }
+
+    it { is_expected.to be false }
+  end
+
+  describe "the candidate behavior" do
+    subject { described_class.new(actor: actor).run(:candidate) }
+
+    it { is_expected.to be true }
+  end
+
+  context "when tracking initial writes" do
+    let!(:project) { create(:project, :repository) }
+
+    def stub_gitaly_count(count = 1)
+      allow(Gitlab::GitalyClient).to receive(:call).and_call_original
+      allow(Gitlab::GitalyClient).to receive(:call).with(anything, :commit_service, :count_commits, anything, anything)
+        .and_return(double(count: count))
+    end
+
+    before do
+      stub_gitaly_count
+    end
+
+    it "tracks an event for the first commit on a project with a repository" do
+      expect(subject).to receive(:track).with(:write, property: project.created_at.to_s, value: 1).and_call_original
+
+      subject.track_initial_writes(project)
+    end
+
+    it "tracks an event for the second commit on a project with a repository" do
+      stub_gitaly_count(2)
+
+      expect(subject).to receive(:track).with(:write, property: project.created_at.to_s, value: 2).and_call_original
+
+      subject.track_initial_writes(project)
+    end
+
+    it "doesn't track if the repository has more then 2 commits" do
+      stub_gitaly_count(3)
+
+      expect(subject).not_to receive(:track)
+
+      subject.track_initial_writes(project)
+    end
+
+    it "doesn't track when we generally shouldn't" do
+      allow(subject).to receive(:should_track?).and_return(false)
+
+      expect(subject).not_to receive(:track)
+
+      subject.track_initial_writes(project)
+    end
+
+    it "doesn't track if the project is older" do
+      expect(project).to receive(:created_at).and_return(described_class::EXPERIMENT_START_DATE - 1.minute)
+
+      expect(subject).not_to receive(:track)
+
+      subject.track_initial_writes(project)
+    end
+
+    it "handles exceptions by logging them" do
+      allow(Gitlab::GitalyClient).to receive(:call).with(anything, :commit_service, :count_commits, anything, anything)
+        .and_raise(e = StandardError.new('_message_'))
+
+      expect(Gitlab::ErrorTracking).to receive(:track_exception).with(e, experiment: 'new_project_readme')
+
+      subject.track_initial_writes(project)
+    end
+  end
+end
diff --git a/spec/experiments/strategy/round_robin_spec.rb b/spec/experiments/strategy/round_robin_spec.rb
new file mode 100644
index 00000000000..f837a4701b2
--- /dev/null
+++ b/spec/experiments/strategy/round_robin_spec.rb
@@ -0,0 +1,68 @@
+# 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