Merge branch 'jj-canary-experiment' into 'master'

Tests to ensure experiments publish and continue working

See merge request gitlab-org/gitlab!58814

.gitlab/CODEOWNERS

@@ -277,6 +277,7 @@ Dangerfile @gl-quality/eng-prod
 /lib/gitlab/experimentation/ @gitlab-org/growth/experiment-devs
 /lib/gitlab/experimentation.rb @gitlab-org/growth/experiment-devs
 /lib/gitlab/experimentation_logger.rb @gitlab-org/growth/experiment-devs
+/ee/spec/requests/api/experiments_spec.rb @gitlab-org/growth/experiment-devs
 
 [Legal]
 /config/dependency_decisions.yml @gitlab-org/legal-reviewers

app/controllers/projects/issues_controller.rb

@@ -69,9 +69,6 @@ class Projects::IssuesController < Projects::ApplicationController
 
   around_action :allow_gitaly_ref_name_caching, only: [:discussions]
 
-  before_action :run_null_hypothesis_experiment,
-                only: [:index, :new, :create]
-
   respond_to :html
 
   alias_method :designs, :show
@@ -400,14 +397,6 @@ class Projects::IssuesController < Projects::ApplicationController
     action_name == 'service_desk'
   end
 
-  def run_null_hypothesis_experiment
-    experiment(:null_hypothesis, project: project) do |e|
-      e.use { } # define the control
-      e.try { } # define the candidate
-      e.track(action_name) # track the action so we can build a funnel
-    end
-  end
-
   # Overridden in EE
   def create_vulnerability_issue_feedback(issue); end
 end

ee/lib/api/experiments.rb

@@ -11,13 +11,20 @@ module API
         success EE::API::Entities::Experiment
       end
       get do
-        experiments = Feature::Definition.definitions.values.select { |d| d.attributes[:type] == 'experiment' }
+        experiments = []
+
+        experiment(:null_hypothesis, canary: true, user: current_user) do |e|
+          e.use { bad_request! 'experimentation may not be working right now' }
+          e.try { experiments = Feature::Definition.definitions.values.select { |d| d.attributes[:type] == 'experiment' } }
+        end
 
         present experiments, with: EE::API::Entities::Experiment, current_user: current_user
       end
     end
 
     helpers do
+      include Gitlab::Experiment::Dsl
+
       def authorize_read_experiments!
         authenticate!
 

ee/spec/requests/api/experiments_spec.rb

@@ -12,12 +12,13 @@ RSpec.describe API::Experiments do
     context 'when on .com' do
       before do
         allow(Gitlab).to receive(:com?).and_return(true)
+        stub_experiments(null_hypothesis: :candidate)
 
         definition = YAML.load_file(definition_yaml).deep_symbolize_keys!
         allow(Feature::Definition.definitions).to receive(:values).and_return([
           Feature::Definition.new(definition_yaml.to_s, definition),
           Feature::Definition.new(
-            "foo/non_experiment.yml",
+            'foo/non_experiment.yml',
             definition.merge(type: 'development', name: 'non_experiment')
           )
         ])
@@ -54,15 +55,15 @@ RSpec.describe API::Experiments do
 
           expect(response).to have_gitlab_http_status(:ok)
           expect(json_response).to include({
-            key: "null_hypothesis",
+            key: 'null_hypothesis',
             state: :off,
             enabled: false,
-            name: "null_hypothesis",
-            introduced_by_url: "https://gitlab.com/gitlab-org/gitlab/-/merge_requests/45840",
+            name: 'null_hypothesis',
+            introduced_by_url: 'https://gitlab.com/gitlab-org/gitlab/-/merge_requests/45840',
             rollout_issue_url: nil,
-            milestone: "13.7",
-            type: "experiment",
-            group: "group::adoption",
+            milestone: '13.7',
+            type: 'experiment',
+            group: 'group::adoption',
             default_enabled: false
           }.as_json)
         end
@@ -76,9 +77,83 @@ RSpec.describe API::Experiments do
           expect(json_response).to include(hash_including({
             state: :conditional,
             enabled: true,
-            name: "null_hypothesis"
+            name: 'null_hypothesis'
           }.as_json))
         end
+
+        describe 'the null_hypothesis as a canary' do
+          # This group of test ensures that we will continue to have a functional
+          # backend experiment. It's part of the suite of tooling that's in place to
+          # ensure that we don't change notable aspects of experimentation, like
+          # the position of the contexts etc.
+          #
+          # We wrap our experiments endpoint in a canary like experiment that if
+          # broken will render this endpoint visibly broken.
+          #
+          # This is something of an integration level and shouldn't be adjusted
+          # without proper consultation with the relevant Growth teams.
+
+          it 'runs and tracks the expected events' do
+            contexts = []
+
+            # Yes, we really do want to test this and the only way to get here
+            # is by calling a private method.
+            expect(Gitlab::Tracking.send(:snowplow)).to receive(:event).with(
+              'null_hypothesis',
+              'assignment',
+              label: nil,
+              property: nil,
+              value: nil,
+              context: [
+                instance_of(SnowplowTracker::SelfDescribingJson),
+                instance_of(SnowplowTracker::SelfDescribingJson)
+              ]
+            ) { |_, _, **options| contexts = options[:context] }
+
+            get api('/experiments', user)
+
+            # Ensure the order of the contexts stays correct for now.
+            #
+            # If you change this, you need to talk with the growth team,
+            # because some reporting is done (incorrectly) based on the index
+            # of this context.
+            expect(contexts[1].to_json).to include({
+              schema: 'iglu:com.gitlab/gitlab_experiment/jsonschema/1-0-0',
+              data: {
+                experiment: 'null_hypothesis',
+                key: anything,
+                variant: 'candidate'
+              }
+            })
+          end
+
+          it 'returns a 400 if experimentation seems broken' do
+            # we assume that rendering control would only be done in error.
+            stub_experiments(null_hypothesis: :control)
+
+            get api('/experiments', user)
+
+            expect(response).to have_gitlab_http_status(:bad_request)
+            expect(json_response).to eq({
+              message: '400 Bad request - experimentation may not be working right now'
+            }.as_json)
+          end
+
+          it 'publishes into a collection of experiments that have been run in the request' do
+            pending 'requires gitlab-experiment >= 0.5.4'
+
+            get api('/experiments', user)
+
+            expect(ApplicationExperiment.published_experiments).to eq(
+              'null_hypothesis' => {
+                excluded: false,
+                experiment: 'null_hypothesis',
+                key: 'abc123',
+                variant: 'candidate'
+              }
+            )
+          end
+        end
       end
     end
 

spec/controllers/projects/issues_controller_spec.rb

@@ -64,24 +64,6 @@ RSpec.describe Projects::IssuesController do
           expect(response).to have_gitlab_http_status(:moved_permanently)
         end
       end
-
-      describe 'the null hypothesis experiment', :experiment do
-        before do
-          stub_experiments(null_hypothesis: :candidate)
-        end
-
-        it 'defines the expected before actions' do
-          expect(controller).to use_before_action(:run_null_hypothesis_experiment)
-        end
-
-        it 'assigns the candidate experience and tracks the event' do
-          expect(experiment(:null_hypothesis)).to track('index').for(:candidate)
-            .with_context(project: project)
-            .on_next_instance
-
-          get :index, params: { namespace_id: project.namespace, project_id: project }
-        end
-      end
     end
 
     context 'internal issue tracker' do

spec/features/admin/admin_abuse_reports_spec.rb

@@ -56,7 +56,7 @@ RSpec.describe "Admin::AbuseReports", :js do
 
     describe 'filtering by user' do
       let!(:user2) { create(:user) }
-      let!(:abuse_report)   { create(:abuse_report, user: user) }
+      let!(:abuse_report) { create(:abuse_report, user: user) }
       let!(:abuse_report_2) { create(:abuse_report, user: user2) }
 
       it 'shows only single user report' do

spec/features/gitlab_experiments_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe "Gitlab::Experiment", :js do
+  # This is part of a set of tests that ensure that tracking remains
+  # consistent at the front end layer. Since we don't want to actually
+  # introduce an experiment in real code, we're going to simulate it
+  # here.
+  let(:user) { create(:user) }
+
+  before do
+    admin = create(:admin)
+    sign_in(admin)
+    gitlab_enable_admin_mode_sign_in(admin)
+    stub_experiments(null_hypothesis: :candidate)
+  end
+
+  describe 'with event tracking' do
+    it 'publishes the experiments that have been run to the client', :experiment do
+      allow_next_instance_of(Admin::AbuseReportsController) do |instance|
+        allow(instance).to receive(:index).and_wrap_original do |original|
+          instance.experiment(:null_hypothesis, user: instance.current_user) do |e|
+            e.use { original.call }
+            e.try { original.call }
+          end
+        end
+      end
+
+      visit admin_abuse_reports_path
+
+      expect(page).to have_content('Abuse Reports')
+
+      published_experiments = page.evaluate_script('window.gon.experiment')
+      expect(published_experiments).to include({
+        'null_hypothesis' => {
+          'experiment' => 'null_hypothesis',
+          'key' => anything,
+          'variant' => 'candidate'
+        }
+      })
+    end
+  end
+end