Merge branch 'jj-update-gitlab-experiment-revert-revert' into 'master'

Update gitlab-experiment to version 0.4.8 [RUN ALL RSPEC]

See merge request gitlab-org/gitlab!52824

Gemfile

@@ -477,7 +477,7 @@ gem 'flipper', '~> 0.17.1'
 gem 'flipper-active_record', '~> 0.17.1'
 gem 'flipper-active_support_cache_store', '~> 0.17.1'
 gem 'unleash', '~> 0.1.5'
-gem 'gitlab-experiment', '~> 0.4.5'
+gem 'gitlab-experiment', '~> 0.4.8'
 
 # Structured logging
 gem 'lograge', '~> 0.5'

Gemfile.lock

@@ -424,7 +424,7 @@ GEM
     github-markup (1.7.0)
     gitlab-chronic (0.10.5)
       numerizer (~> 0.2)
-    gitlab-experiment (0.4.5)
+    gitlab-experiment (0.4.8)
       activesupport (>= 3.0)
       scientist (~> 1.5, >= 1.5.0)
     gitlab-fog-azure-rm (1.0.0)
@@ -1364,7 +1364,7 @@ DEPENDENCIES
   gitaly (~> 13.8.0.pre.rc3)
   github-markup (~> 1.7.0)
   gitlab-chronic (~> 0.10.5)
-  gitlab-experiment (~> 0.4.5)
+  gitlab-experiment (~> 0.4.8)
   gitlab-fog-azure-rm (~> 1.0)
   gitlab-labkit (= 0.14.0)
   gitlab-license (~> 1.0)

app/controllers/projects/issues_controller.rb

@@ -59,8 +59,7 @@ class Projects::IssuesController < Projects::ApplicationController
   around_action :allow_gitaly_ref_name_caching, only: [:discussions]
 
   before_action :run_null_hypothesis_experiment,
-                only: [:index, :new, :create],
-                if: -> { Feature.enabled?(:gitlab_experiments) }
+                only: [:index, :new, :create]
 
   respond_to :html
 

app/experiments/application_experiment.rb

 # frozen_string_literal: true
 
 class ApplicationExperiment < Gitlab::Experiment
+  def enabled?
+    return false if Feature::Definition.get(name).nil? # there has to be a feature flag yaml file
+    return false unless Gitlab.dev_env_or_com? # we're in an environment that allows experiments
+
+    Feature.get(name).state != :off # rubocop:disable Gitlab/AvoidFeatureGet
+  end
+
   def publish(_result)
     track(:assignment) # track that we've assigned a variant for this context
     Gon.global.push({ experiment: { name => signature } }, true) # push to client
   end
 
   def track(action, **event_args)
-    return if excluded? # no events for opted out actors or excluded subjects
+    return unless should_track? # no events for opted out actors or excluded subjects
 
     Gitlab::Tracking.event(name, action.to_s, **event_args.merge(
       context: (event_args[:context] || []) << SnowplowTracker::SelfDescribingJson.new(

config/feature_flags/development/gitlab_experiments.yml

----
-name: gitlab_experiments
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/45840
-rollout_issue_url: 
-milestone: '13.7'
-type: development
-group: group::adoption
-default_enabled: false

spec/controllers/projects/issues_controller_spec.rb

@@ -63,56 +63,23 @@ RSpec.describe Projects::IssuesController do
         end
       end
 
-      describe 'the null hypothesis experiment', :snowplow do
-        it 'defines the expected before actions' do
-          expect(controller).to use_before_action(:run_null_hypothesis_experiment)
-        end
-
-        context 'when rolled out to 100%' do
-          it 'assigns the candidate experience and tracks the event' do
-            get :index, params: { namespace_id: project.namespace, project_id: project }
-
-            expect_snowplow_event(
-              category: 'null_hypothesis',
-              action: 'index',
-              context: [{
-                schema: 'iglu:com.gitlab/gitlab_experiment/jsonschema/0-3-0',
-                data: { variant: 'candidate', experiment: 'null_hypothesis', key: anything }
-              }]
-            )
-          end
-        end
-
-        context 'when not rolled out' do
+      describe 'the null hypothesis experiment', :experiment do
         before do
-            stub_feature_flags(null_hypothesis: false)
+          stub_experiments(null_hypothesis: :candidate)
         end
 
-          it 'assigns the control experience and tracks the event' do
-            get :index, params: { namespace_id: project.namespace, project_id: project }
-
-            expect_snowplow_event(
-              category: 'null_hypothesis',
-              action: 'index',
-              context: [{
-                schema: 'iglu:com.gitlab/gitlab_experiment/jsonschema/0-3-0',
-                data: { variant: 'control', experiment: 'null_hypothesis', key: anything }
-              }]
-            )
-          end
+        it 'defines the expected before actions' do
+          expect(controller).to use_before_action(:run_null_hypothesis_experiment)
         end
 
-        context 'when gitlab_experiments is disabled' do
-          it 'does not run the experiment at all' do
-            stub_feature_flags(gitlab_experiments: false)
-
-            expect(controller).not_to receive(:run_null_hypothesis_experiment)
+        it 'assigns the candidate experience and tracks the event' do
+          expect(experiment(:null_hypothesis)).to track('index').on_any_instance.for(:candidate)
+            .with_context(project: project)
 
           get :index, params: { namespace_id: project.namespace, project_id: project }
         end
       end
     end
-    end
 
     context 'internal issue tracker' do
       before do

spec/experiments/application_experiment_spec.rb

@@ -2,15 +2,51 @@
 
 require 'spec_helper'
 
-RSpec.describe ApplicationExperiment do
+RSpec.describe ApplicationExperiment, :experiment do
   subject { described_class.new(:stub) }
 
+  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('stub')
 
     described_class.new(: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('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('stub').and_return(double(state: :off))
+
+      expect(subject).not_to be_enabled
+    end
+  end
+
   describe "publishing results" do
     it "tracks the assignment" do
       expect(subject).to receive(:track).with(:assignment)
@@ -37,8 +73,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)
 

spec/support/gitlab_experiment.rb

 # frozen_string_literal: true
 
+# Require the provided spec helper and matchers.
+require 'gitlab/experiment/rspec'
+
 # This is a temporary fix until we have a larger discussion around the
 # challenges raised in https://gitlab.com/gitlab-org/gitlab/-/issues/300104
 class ApplicationExperiment < Gitlab::Experiment # rubocop:disable Gitlab/NamespacedClass