Add latest changes from gitlab-org/gitlab@master

4 files changed, 91 insertions, 19 deletions

diff --git a/spec/lib/atlassian/jira_connect/client_spec.rb b/spec/lib/atlassian/jira_connect/client_spec.rb
index 257174aad4f..63f6573818b 100644
--- a/spec/lib/atlassian/jira_connect/client_spec.rb
+++ b/spec/lib/atlassian/jira_connect/client_spec.rb
@@ -368,24 +368,6 @@ RSpec.describe Atlassian::JiraConnect::Client do
       subject.send(:store_build_info, project: project, pipelines: pipelines.take(1))
     end
 
-    it 'does not call the API if the feature flag is not enabled' do
-      stub_feature_flags(jira_sync_builds: false)
-
-      expect(subject).not_to receive(:post)
-
-      subject.send(:store_build_info, project: project, pipelines: pipelines)
-    end
-
-    it 'does call the API if the feature flag enabled for the project' do
-      stub_feature_flags(jira_sync_builds: project)
-
-      expect(subject).to receive(:post)
-        .with('/rest/builds/0.1/bulk', { builds: Array })
-        .and_call_original
-
-      subject.send(:store_build_info, project: project, pipelines: pipelines)
-    end
-
     context 'there are errors' do
       let(:failures) do
         [{ errors: [{ message: 'X' }, { message: 'Y' }] }, { errors: [{ message: 'Z' }] }]
diff --git a/spec/lib/gitlab/experimentation/controller_concern_spec.rb b/spec/lib/gitlab/experimentation/controller_concern_spec.rb
index c47f71c207d..1cebe37bea5 100644
--- a/spec/lib/gitlab/experimentation/controller_concern_spec.rb
+++ b/spec/lib/gitlab/experimentation/controller_concern_spec.rb
@@ -10,6 +10,10 @@ RSpec.describe Gitlab::Experimentation::ControllerConcern, type: :controller do
           use_backwards_compatible_subject_index: true
         },
         test_experiment: {
+          tracking_category: 'Team',
+          rollout_strategy: rollout_strategy
+        },
+        my_experiment: {
           tracking_category: 'Team'
         }
       }
@@ -20,6 +24,7 @@ RSpec.describe Gitlab::Experimentation::ControllerConcern, type: :controller do
   end
 
   let(:enabled_percentage) { 10 }
+  let(:rollout_strategy) { nil }
 
   controller(ApplicationController) do
     include Gitlab::Experimentation::ControllerConcern
@@ -117,6 +122,7 @@ RSpec.describe Gitlab::Experimentation::ControllerConcern, type: :controller do
       end
 
       context 'when subject is given' do
+        let(:rollout_strategy) { :user }
         let(:user) { build(:user) }
 
         it 'uses the subject' do
@@ -244,6 +250,7 @@ RSpec.describe Gitlab::Experimentation::ControllerConcern, type: :controller do
 
         it "provides the subject's hashed global_id as label" do
           experiment_subject = double(:subject, to_global_id: 'abc')
+          allow(Gitlab::Experimentation).to receive(:valid_subject_for_rollout_strategy?).and_return(true)
 
           controller.track_experiment_event(:test_experiment, 'start', 1, subject: experiment_subject)
 
@@ -420,6 +427,26 @@ RSpec.describe Gitlab::Experimentation::ControllerConcern, type: :controller do
 
           controller.record_experiment_user(:test_experiment, context)
         end
+
+        context 'with a cookie based rollout strategy' do
+          it 'calls tracking_group with a nil subject' do
+            expect(controller).to receive(:tracking_group).with(:test_experiment, nil, subject: nil).and_return(:experimental)
+            allow(::Experiment).to receive(:add_user).with(:test_experiment, :experimental, user, context)
+
+            controller.record_experiment_user(:test_experiment, context)
+          end
+        end
+
+        context 'with a user based rollout strategy' do
+          let(:rollout_strategy) { :user }
+
+          it 'calls tracking_group with a user subject' do
+            expect(controller).to receive(:tracking_group).with(:test_experiment, nil, subject: user).and_return(:experimental)
+            allow(::Experiment).to receive(:add_user).with(:test_experiment, :experimental, user, context)
+
+            controller.record_experiment_user(:test_experiment, context)
+          end
+        end
       end
 
       context 'the user is part of the control group' do
diff --git a/spec/lib/gitlab/experimentation/experiment_spec.rb b/spec/lib/gitlab/experimentation/experiment_spec.rb
index 008e6699597..94dbf1d7e4b 100644
--- a/spec/lib/gitlab/experimentation/experiment_spec.rb
+++ b/spec/lib/gitlab/experimentation/experiment_spec.rb
@@ -9,7 +9,8 @@ RSpec.describe Gitlab::Experimentation::Experiment do
   let(:params) do
     {
       tracking_category: 'Category1',
-      use_backwards_compatible_subject_index: true
+      use_backwards_compatible_subject_index: true,
+      rollout_strategy: nil
     }
   end
 
diff --git a/spec/lib/gitlab/experimentation_spec.rb b/spec/lib/gitlab/experimentation_spec.rb
index b503960b8c7..726d2380446 100644
--- a/spec/lib/gitlab/experimentation_spec.rb
+++ b/spec/lib/gitlab/experimentation_spec.rb
@@ -27,6 +27,8 @@ RSpec.describe Gitlab::Experimentation::EXPERIMENTS do
 end
 
 RSpec.describe Gitlab::Experimentation do
+  using RSpec::Parameterized::TableSyntax
+
   before do
     stub_const('Gitlab::Experimentation::EXPERIMENTS', {
       backwards_compatible_test_experiment: {
@@ -35,6 +37,10 @@ RSpec.describe Gitlab::Experimentation do
       },
       test_experiment: {
         tracking_category: 'Team'
+      },
+      tabular_experiment: {
+        tracking_category: 'Team',
+        rollout_strategy: rollout_strategy
       }
     })
 
@@ -46,6 +52,7 @@ RSpec.describe Gitlab::Experimentation do
   end
 
   let(:enabled_percentage) { 10 }
+  let(:rollout_strategy) { nil }
 
   describe '.get_experiment' do
     subject { described_class.get_experiment(:test_experiment) }
@@ -175,4 +182,59 @@ RSpec.describe Gitlab::Experimentation do
       end
     end
   end
+
+  describe '.log_invalid_rollout' do
+    subject { described_class.log_invalid_rollout(:test_experiment, 1) }
+
+    before do
+      allow(described_class).to receive(:valid_subject_for_rollout_strategy?).and_return(valid)
+    end
+
+    context 'subject is not valid for experiment' do
+      let(:valid) { false }
+
+      it 'logs a warning message' do
+        expect_next_instance_of(Gitlab::ExperimentationLogger) do |logger|
+          expect(logger)
+            .to receive(:warn)
+                  .with(
+                    message: 'Subject must conform to the rollout strategy',
+                    experiment_key: :test_experiment,
+                    subject: 'Integer',
+                    rollout_strategy: :cookie
+                  )
+        end
+
+        subject
+      end
+    end
+
+    context 'subject is valid for experiment' do
+      let(:valid) { true }
+
+      it 'does not log a warning message' do
+        expect(Gitlab::ExperimentationLogger).not_to receive(:build)
+
+        subject
+      end
+    end
+  end
+
+  describe '.valid_subject_for_rollout_strategy?' do
+    subject { described_class.valid_subject_for_rollout_strategy?(:tabular_experiment, experiment_subject) }
+
+    where(:rollout_strategy, :experiment_subject, :result) do
+      :cookie | nil       | true
+      nil     | nil       | true
+      :cookie | 'string'  | true
+      nil     | User.new  | false
+      :user   | User.new  | true
+      :group  | User.new  | false
+      :group  | Group.new | true
+    end
+
+    with_them do
+      it { is_expected.to be(result) }
+    end
+  end
 end